core/modules/outside_in/css/outside_in.motion.css | 4 +- core/modules/outside_in/css/outside_in.theme.css | 6 +- core/modules/outside_in/js/outside_in.es6.js | 6 +- core/modules/outside_in/js/outside_in.js | 6 +- core/modules/outside_in/outside_in.api.php | 56 +++++++++++ core/modules/outside_in/outside_in.module | 64 ++++++++++-- core/modules/outside_in/outside_in.routing.yml | 1 + core/modules/outside_in/outside_in.services.yml | 5 + .../BlockPluginHasOffCanvasFormAccessCheck.php | 49 ++++++++++ .../outside_in_test/outside_in_test.info.yml | 9 ++ .../OffCanvasFormAnntationIsClassBlockForm.php | 44 +++++++++ .../Block/OffCanvasFormAnnotationIsClassBlock.php | 27 ++++++ .../Block/OffCanvasFormAnnotationIsFalseBlock.php | 27 ++++++ .../Block/OffCanvasFormAnnotationNoneBlock.php | 24 +++++ .../tests/src/Functional/OutsideInTest.php | 108 +++++++++++++++++++++ .../OutsideInBlockFormTest.php | 23 +++++ .../BlockPluginHasOffCanvasFormAccessCheckTest.php | 100 +++++++++++++++++++ 17 files changed, 538 insertions(+), 21 deletions(-) diff --git a/core/modules/outside_in/css/outside_in.motion.css b/core/modules/outside_in/css/outside_in.motion.css index 5c27ef1..a7220e1 100644 --- a/core/modules/outside_in/css/outside_in.motion.css +++ b/core/modules/outside_in/css/outside_in.motion.css @@ -19,8 +19,8 @@ /* Transition the editables on the page, their contextual links and their hover states. */ .dialog-off-canvas__main-canvas .contextual, -.dialog-off-canvas__main-canvas .js-outside-in-edit-mode .outside-in-editable, -.dialog-off-canvas__main-canvas.js-tray-open .js-outside-in-edit-mode .outside-in-editable { +.dialog-off-canvas__main-canvas .js-outside-in-edit-mode .contextual-region.outside-in-editable, +.dialog-off-canvas__main-canvas.js-tray-open .js-outside-in-edit-mode .contextual-region.outside-in-editable { -webkit-transition: all .7s ease; -moz-transition: all .7s ease; transition: all .7s ease; diff --git a/core/modules/outside_in/css/outside_in.theme.css b/core/modules/outside_in/css/outside_in.theme.css index 0db7a3c..01c974b 100644 --- a/core/modules/outside_in/css/outside_in.theme.css +++ b/core/modules/outside_in/css/outside_in.theme.css @@ -60,12 +60,12 @@ } /* Style the editables while in edit mode. */ -.dialog-off-canvas__main-canvas.js-outside-in-edit-mode .outside-in-editable { +.dialog-off-canvas__main-canvas.js-outside-in-edit-mode .contextual-region.outside-in-editable { outline: 1px dashed rgba(0,0,0,0.5); box-shadow: 0 0 0 1px rgba(255,255,255,0.7); } -.dialog-off-canvas__main-canvas.js-outside-in-edit-mode .outside-in-editable:hover, -.dialog-off-canvas__main-canvas.js-outside-in-edit-mode .outside-in-editable.outside-in-active-editable { +.dialog-off-canvas__main-canvas.js-outside-in-edit-mode .contextual-region.outside-in-editable:hover, +.dialog-off-canvas__main-canvas.js-outside-in-edit-mode .contextual-region.outside-in-editable.outside-in-active-editable { background-color: rgba(0,0,0,0.2); } diff --git a/core/modules/outside_in/js/outside_in.es6.js b/core/modules/outside_in/js/outside_in.es6.js index 410b560..25c1902 100644 --- a/core/modules/outside_in/js/outside_in.es6.js +++ b/core/modules/outside_in/js/outside_in.es6.js @@ -8,7 +8,7 @@ (function ($, Drupal) { const blockConfigureSelector = '[data-outside-in-edit]'; const toggleEditSelector = '[data-drupal-outsidein="toggle"]'; - const itemsToToggleSelector = '[data-off-canvas-main-canvas], #toolbar-bar, [data-drupal-outsidein="editable"] a, [data-drupal-outsidein="editable"] button'; + const itemsToToggleSelector = '[data-off-canvas-main-canvas], #toolbar-bar, .contextual-region[data-drupal-outsidein="editable"] a, .contextual-region[data-drupal-outsidein="editable"] button'; const contextualItemsSelector = '[data-contextual-id] a, [data-contextual-id] button'; const quickEditItemSelector = '[data-quickedit-entity-id]'; @@ -134,7 +134,7 @@ $editButton.text(Drupal.t('Editing')); closeToolbarTrays(); - $editables = $('[data-drupal-outsidein="editable"]').once('outsidein'); + $editables = $('.contextual-region[data-drupal-outsidein="editable"]').once('outsidein'); if ($editables.length) { // Use event capture to prevent clicks on links. document.querySelector('[data-off-canvas-main-canvas]').addEventListener('click', preventClick, true); @@ -168,7 +168,7 @@ } // Disable edit mode. else { - $editables = $('[data-drupal-outsidein="editable"]').removeOnce('outsidein'); + $editables = $('.contextual-region[data-drupal-outsidein="editable"]').removeOnce('outsidein'); if ($editables.length) { document.querySelector('[data-off-canvas-main-canvas]').removeEventListener('click', preventClick, true); $editables.off('.outsidein'); diff --git a/core/modules/outside_in/js/outside_in.js b/core/modules/outside_in/js/outside_in.js index 95cf90e..350ded0 100644 --- a/core/modules/outside_in/js/outside_in.js +++ b/core/modules/outside_in/js/outside_in.js @@ -8,7 +8,7 @@ (function ($, Drupal) { var blockConfigureSelector = '[data-outside-in-edit]'; var toggleEditSelector = '[data-drupal-outsidein="toggle"]'; - var itemsToToggleSelector = '[data-off-canvas-main-canvas], #toolbar-bar, [data-drupal-outsidein="editable"] a, [data-drupal-outsidein="editable"] button'; + var itemsToToggleSelector = '[data-off-canvas-main-canvas], #toolbar-bar, .contextual-region[data-drupal-outsidein="editable"] a, .contextual-region[data-drupal-outsidein="editable"] button'; var contextualItemsSelector = '[data-contextual-id] a, [data-contextual-id] button'; var quickEditItemSelector = '[data-quickedit-entity-id]'; @@ -74,7 +74,7 @@ $editButton.text(Drupal.t('Editing')); closeToolbarTrays(); - $editables = $('[data-drupal-outsidein="editable"]').once('outsidein'); + $editables = $('.contextual-region[data-drupal-outsidein="editable"]').once('outsidein'); if ($editables.length) { document.querySelector('[data-off-canvas-main-canvas]').addEventListener('click', preventClick, true); @@ -97,7 +97,7 @@ }); } } else { - $editables = $('[data-drupal-outsidein="editable"]').removeOnce('outsidein'); + $editables = $('.contextual-region[data-drupal-outsidein="editable"]').removeOnce('outsidein'); if ($editables.length) { document.querySelector('[data-off-canvas-main-canvas]').removeEventListener('click', preventClick, true); $editables.off('.outsidein'); diff --git a/core/modules/outside_in/outside_in.api.php b/core/modules/outside_in/outside_in.api.php new file mode 100644 index 0000000..913036f --- /dev/null +++ b/core/modules/outside_in/outside_in.api.php @@ -0,0 +1,56 @@ +createInstance($variables['plugin_id']); + if ($access_checker->accessBlockPlugin($block_plugin)->isAllowed()) { // Add class and attributes to all blocks to allow Javascript to target. $variables['attributes']['class'][] = 'outside-in-editable'; $variables['attributes']['data-drupal-outsidein'] = 'editable'; @@ -139,17 +153,47 @@ function outside_in_toolbar_alter(&$items) { /** * Implements hook_block_alter(). + * + * Ensures every block plugin definition has an 'off_canvas' form specified. + * + * @see \Drupal\outside_in\Access\BlockPluginHasOffCanvasFormAccessCheck */ function outside_in_block_alter(&$definitions) { - if (!empty($definitions['system_branding_block'])) { - $definitions['system_branding_block']['forms']['off_canvas'] = SystemBrandingOffCanvasForm::class; - } - - // Since menu blocks use derivatives, check the definition ID instead of - // relying on the plugin ID. foreach ($definitions as &$definition) { - if ($definition['id'] === 'system_menu_block') { - $definition['forms']['off_canvas'] = SystemMenuOffCanvasForm::class; + // If the block plugin already defines an 'off_canvas' form, there's nothing + // to do. + if (isset($definition['forms']['off_canvas'])) { + continue; + } + + switch ($definition['id']) { + // Use specialized off-canvas forms when they're available. + // @todo move these into the corresponding block plugin annotations in https://www.drupal.org/node/2896356 + case 'system_menu_block': + $definition['forms']['off_canvas'] = SystemMenuOffCanvasForm::class; + break; + case 'system_branding_block': + $definition['forms']['off_canvas'] = SystemBrandingOffCanvasForm::class; + break; + + // No off-canvas form for the page title block, despite it having + // contextual links: it's too confusing that you're editing configuration, + // not content, so the title itself cannot actually be changed. + // @todo move these into the corresponding block plugin annotations in https://www.drupal.org/node/2896356 + case 'page_title_block': + $definition['forms']['off_canvas'] = FALSE; + break; + case 'system_main_block': + $definition['forms']['off_canvas'] = FALSE; + break; + case 'help_block': + $definition['forms']['off_canvas'] = FALSE; + break; + + // Otherwise fall back to the built-in form for the block plugin. + default: + $definition['forms']['off_canvas'] = $definition['class']; + break; } } } diff --git a/core/modules/outside_in/outside_in.routing.yml b/core/modules/outside_in/outside_in.routing.yml index 18a564c..a778727 100644 --- a/core/modules/outside_in/outside_in.routing.yml +++ b/core/modules/outside_in/outside_in.routing.yml @@ -5,3 +5,4 @@ entity.block.off_canvas_form: _title_callback: '\Drupal\outside_in\Block\BlockEntityOffCanvasForm::title' requirements: _permission: 'administer blocks' + _access_block_plugin_has_offcanvas_form: 'TRUE' diff --git a/core/modules/outside_in/outside_in.services.yml b/core/modules/outside_in/outside_in.services.yml index ce82146..5d95c1e 100644 --- a/core/modules/outside_in/outside_in.services.yml +++ b/core/modules/outside_in/outside_in.services.yml @@ -4,3 +4,8 @@ services: arguments: ['@title_resolver', '@renderer'] tags: - { name: render.main_content_renderer, format: drupal_dialog.off_canvas } + + access_check.outside_in.block.off_canvas_form: + class: Drupal\outside_in\Access\BlockPluginHasOffCanvasFormAccessCheck + tags: + - { name: access_check, applies_to: _access_block_plugin_has_offcanvas_form } diff --git a/core/modules/outside_in/src/Access/BlockPluginHasOffCanvasFormAccessCheck.php b/core/modules/outside_in/src/Access/BlockPluginHasOffCanvasFormAccessCheck.php new file mode 100644 index 0000000..e8136d6 --- /dev/null +++ b/core/modules/outside_in/src/Access/BlockPluginHasOffCanvasFormAccessCheck.php @@ -0,0 +1,49 @@ +getPlugin(); + return $this->accessBlockPlugin($block_plugin); + } + + /** + * Checks access for accessing a block plugin's 'off_canvas' form. + * + * @param \Drupal\Core\Block\BlockPluginInterface $block_plugin + * The block plugin whose 'off_canvas' form is being accessed. + * + * @return \Drupal\Core\Access\AccessResultInterface + * The access result. + * + * @see outside_in_preprocess_block() + * @todo Remove when outside_in_preprocess_block() is removed. + */ + public function accessBlockPlugin(BlockPluginInterface $block_plugin) { + return AccessResult::allowedIf($block_plugin instanceof PluginWithFormsInterface && $block_plugin->hasFormClass('off_canvas')); + } + +} diff --git a/core/modules/outside_in/tests/modules/outside_in_test/outside_in_test.info.yml b/core/modules/outside_in/tests/modules/outside_in_test/outside_in_test.info.yml new file mode 100644 index 0000000..590f559 --- /dev/null +++ b/core/modules/outside_in/tests/modules/outside_in_test/outside_in_test.info.yml @@ -0,0 +1,9 @@ +name: 'Settings Tray Test' +type: module +description: 'Provides Settings Tray test functionality.' +package: Testing +version: VERSION +core: 8.x +dependencies: + - block + - outside_in diff --git a/core/modules/outside_in/tests/modules/outside_in_test/src/Form/OffCanvasFormAnntationIsClassBlockForm.php b/core/modules/outside_in/tests/modules/outside_in_test/src/Form/OffCanvasFormAnntationIsClassBlockForm.php new file mode 100644 index 0000000..2136615 --- /dev/null +++ b/core/modules/outside_in/tests/modules/outside_in_test/src/Form/OffCanvasFormAnntationIsClassBlockForm.php @@ -0,0 +1,44 @@ +plugin->buildConfigurationForm($form, $form_state); + + $form['some_setting'] = [ + '#type' => 'select', + '#title' => t('Some setting'), + '#options' => [ + 'a' => 'A', + 'b' => 'B', + ], + '#required' => TRUE, + ]; + + return $form; + } + + /** + * {@inheritdoc} + */ + public function submitConfigurationForm(array &$form, FormStateInterface $form_state) {} + +} diff --git a/core/modules/outside_in/tests/modules/outside_in_test/src/Plugin/Block/OffCanvasFormAnnotationIsClassBlock.php b/core/modules/outside_in/tests/modules/outside_in_test/src/Plugin/Block/OffCanvasFormAnnotationIsClassBlock.php new file mode 100644 index 0000000..41af9dc --- /dev/null +++ b/core/modules/outside_in/tests/modules/outside_in_test/src/Plugin/Block/OffCanvasFormAnnotationIsClassBlock.php @@ -0,0 +1,27 @@ + 'class']; + } + +} diff --git a/core/modules/outside_in/tests/modules/outside_in_test/src/Plugin/Block/OffCanvasFormAnnotationIsFalseBlock.php b/core/modules/outside_in/tests/modules/outside_in_test/src/Plugin/Block/OffCanvasFormAnnotationIsFalseBlock.php new file mode 100644 index 0000000..6458fa1 --- /dev/null +++ b/core/modules/outside_in/tests/modules/outside_in_test/src/Plugin/Block/OffCanvasFormAnnotationIsFalseBlock.php @@ -0,0 +1,27 @@ + 'FALSE']; + } + +} diff --git a/core/modules/outside_in/tests/modules/outside_in_test/src/Plugin/Block/OffCanvasFormAnnotationNoneBlock.php b/core/modules/outside_in/tests/modules/outside_in_test/src/Plugin/Block/OffCanvasFormAnnotationNoneBlock.php new file mode 100644 index 0000000..1e67f23 --- /dev/null +++ b/core/modules/outside_in/tests/modules/outside_in_test/src/Plugin/Block/OffCanvasFormAnnotationNoneBlock.php @@ -0,0 +1,24 @@ + 'none']; + } + +} diff --git a/core/modules/outside_in/tests/src/Functional/OutsideInTest.php b/core/modules/outside_in/tests/src/Functional/OutsideInTest.php new file mode 100644 index 0000000..1cefffa --- /dev/null +++ b/core/modules/outside_in/tests/src/Functional/OutsideInTest.php @@ -0,0 +1,108 @@ +id(); + } + + /** + * Tests the three possible forms[off_canvas] annotations: class, FALSE, none. + * + * There is also functional JS test coverage to ensure that the two blocks + * that support Settings Tray (the "class" & "none" cases) do work correctly. + * + * @see OutsideInBlockFormTest::testBlocks() + */ + public function testPossibleAnnotations() { + $test_block_plugin_ids = [ + // Block that explicitly provides an "off_canvas" form class. + 'outside_in_test_class', + // Block that explicitly provides no "off_canvas" form, thus opting out. + 'outside_in_test_false', + // Block that does nothing explicit for Settings Tray. + 'outside_in_test_none', + ]; + + $placed_blocks = []; + foreach ($test_block_plugin_ids as $plugin_id) { + $placed_blocks[$plugin_id] = $this->placeBlock($plugin_id); + } + + $this->drupalGet(''); + $web_assert = $this->assertSession(); + foreach ($placed_blocks as $plugin_id => $placed_block) { + $block_selector = $this->getBlockSelector($placed_block); + + // All blocks are rendered. + $web_assert->elementExists('css', $block_selector); + + // All blocks except 'outside_in_test_false' are editable. For more + // detailed test coverage, which requires JS execution, see + // OutsideInBlockFormTest::testBlocks(). + if ($plugin_id !== 'outside_in_test_false') { + $web_assert->elementExists('css', "{$block_selector}[data-drupal-outsidein=\"editable\"]"); + } + else { + $web_assert->elementNotExists('css', "{$block_selector}[data-drupal-outsidein=\"editable\"]"); + } + } + } + + /** + * Tests that certain blocks opt out from Settings Tray. + */ + public function testOptOut() { + $web_assert = $this->assertSession(); + + $non_excluded_block = $this->placeBlock('system_powered_by_block'); + $excluded_block_plugin_ids = ['page_title_block', 'system_main_block', 'outside_in_test_false']; + $block_selectors = []; + // Place blocks that should be excluded. + foreach ($excluded_block_plugin_ids as $excluded_block_plugin_id) { + // The block HTML 'id' attribute will be "block-[block_id]". + $block_selectors[] = $this->getBlockSelector($this->placeBlock($excluded_block_plugin_id)); + } + $this->drupalGet(''); + // Assert that block has been marked as "editable" and contextual that + // should exist does. + $web_assert->elementExists('css', $this->getBlockSelector($non_excluded_block) . "[data-drupal-outsidein=\"editable\"]"); + // Assert that each block that has a "forms[off_canvas] = FALSE" annotation: + // - is still rendered on the page + // - but is not marked as "editable" by outside_in_preprocess_block() + // - and does not have the Settings Tray contextual link + foreach ($block_selectors as $block_selector) { + $web_assert->elementExists('css', $block_selector); + $web_assert->elementNotExists('css', "{$block_selector}[data-drupal-outsidein=\"editable\"]"); + $web_assert->elementNotExists('css', "$block_selector [data-outside-in-edit]"); + } + } + +} diff --git a/core/modules/outside_in/tests/src/FunctionalJavascript/OutsideInBlockFormTest.php b/core/modules/outside_in/tests/src/FunctionalJavascript/OutsideInBlockFormTest.php index 04fc230..191010d 100644 --- a/core/modules/outside_in/tests/src/FunctionalJavascript/OutsideInBlockFormTest.php +++ b/core/modules/outside_in/tests/src/FunctionalJavascript/OutsideInBlockFormTest.php @@ -35,6 +35,7 @@ class OutsideInBlockFormTest extends OutsideInJavascriptTestBase { // Add test module to override CSS pointer-events properties because they // cause test failures. 'outside_in_test_css', + 'outside_in_test', ]; /** @@ -109,6 +110,10 @@ public function testBlocks($block_plugin, $new_page_text, $element_selector, $la // Fill out form, save the form. $page->fillField('settings[site_information][site_name]', $new_page_text); break; + + case 'outside_in_test_class': + $web_assert->elementExists('css', '[data-drupal-selector="edit-settings-some-setting"]'); + break; } if (isset($new_page_text)) { @@ -176,6 +181,24 @@ public function providerTestBlocks() { 'button_text' => 'Save Search form', 'toolbar_item' => NULL, ], + // This is the functional JS test coverage accompanying testPossibleAnnotations(). + '\Drupal\outside_in_test\Plugin\Block\OffCanvasFormAnnotationIsClassBlock' => [ + 'block_plugin' => 'outside_in_test_class', + 'new_page_text' => NULL, + 'element_selector' => 'span', + 'label_selector' => NULL, + 'button_text' => NULL, + 'toolbar_item' => NULL, + ], + // This is the functional JS test coverage accompanying testPossibleAnnotations(). + '\Drupal\outside_in_test\Plugin\Block\OffCanvasFormAnnotationNoneBlock' => [ + 'block_plugin' => 'outside_in_test_none', + 'new_page_text' => NULL, + 'element_selector' => 'span', + 'label_selector' => NULL, + 'button_text' => NULL, + 'toolbar_item' => NULL, + ], ]; return $blocks; } diff --git a/core/modules/outside_in/tests/src/Unit/Access/BlockPluginHasOffCanvasFormAccessCheckTest.php b/core/modules/outside_in/tests/src/Unit/Access/BlockPluginHasOffCanvasFormAccessCheckTest.php new file mode 100644 index 0000000..a8d0aab --- /dev/null +++ b/core/modules/outside_in/tests/src/Unit/Access/BlockPluginHasOffCanvasFormAccessCheckTest.php @@ -0,0 +1,100 @@ +randomMachineName(), $plugin_definition); + $block = $this->prophesize(BlockInterface::class); + $block->getPlugin()->willReturn($block_plugin); + + $access_check = new BlockPluginHasOffCanvasFormAccessCheck(); + $this->assertEquals($expected_access_result, $access_check->access($block->reveal())); + $this->assertEquals($expected_access_result, $access_check->accessBlockPlugin($block_plugin)); + } + + public function providerTestAccess() { + $annotation_forms_off_canvas_class = [ + 'provider' => 'block_test', + 'forms' => [ + 'off_canvas' => $this->randomMachineName(), + ], + ]; + $annotation_forms_off_canvas_not_set = [ + 'provider' => 'block_test', + ]; + $annotation_forms_off_canvas_false = [ + 'provider' => 'block_test', + 'forms' => [ + 'off_canvas' => FALSE, + ], + ]; + return [ + 'block plugin with forms, forms[off_canvas] set to class' => [TestBlockClassWithForms::class, $annotation_forms_off_canvas_class, new AccessResultAllowed()], + 'block plugin with forms, forms[off_canvas] not set' => [TestBlockClassWithForms::class, $annotation_forms_off_canvas_not_set, new AccessResultNeutral()], + 'block plugin with forms, forms[off_canvas] set to FALSE' => [TestBlockClassWithForms::class, $annotation_forms_off_canvas_false, new AccessResultNeutral()], + // In practice, all block plugins extend BlockBase, which means they all + // implement PluginWithFormsInterface, but this may change in the future. + // This ensures Settings Tray will continue to work correctly. + 'block plugin without forms, forms[off_canvas] set to class' => [TestBlockClassWithoutForms::class, $annotation_forms_off_canvas_class, new AccessResultNeutral()], + 'block plugin without forms, forms[off_canvas] not set' => [TestBlockClassWithoutForms::class, $annotation_forms_off_canvas_not_set, new AccessResultNeutral()], + 'block plugin without forms, forms[off_canvas] set to FALSE' => [TestBlockClassWithoutForms::class, $annotation_forms_off_canvas_false, new AccessResultNeutral()], + ]; + } + +} + +class TestBlockClassWithForms extends BlockBase { + + public function build() { + return []; + } + +} + +class TestBlockClassWithoutForms implements BlockPluginInterface { + + use UnchangingCacheableDependencyTrait; + public function label() {} + public function access(AccountInterface $account, $return_as_object = FALSE) {} + public function build() {} + public function setConfigurationValue($key, $value) {} + public function blockForm($form, FormStateInterface $form_state) {} + public function blockValidate($form, FormStateInterface $form_state) {} + public function blockSubmit($form, FormStateInterface $form_state) {} + public function getMachineNameSuggestion() {} + public function getConfiguration() {} + public function setConfiguration(array $configuration) {} + public function defaultConfiguration() {} + public function buildConfigurationForm(array $form, FormStateInterface $form_state) {} + public function validateConfigurationForm(array &$form, FormStateInterface $form_state) {} + public function submitConfigurationForm(array &$form, FormStateInterface $form_state) {} + public function getPluginId() {} + public function getPluginDefinition() {} + public function getBaseId() {} + public function getDerivativeId() {} + public function calculateDependencies() {} + +}