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 00000000000..60cfe87a9dc --- /dev/null +++ b/core/modules/outside_in/outside_in.api.php @@ -0,0 +1,61 @@ +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'; @@ -108,7 +120,9 @@ function outside_in_preprocess_block(&$variables) { * Alters the 'contextual' toolbar tab if it exists (meaning the user is allowed * to use contextual links) and if they can administer blocks. * - * @todo Remove the "administer blocks" requirement in https://www.drupal.org/node/2822965 + * @todo Remove the "administer blocks" requirement in + * https://www.drupal.org/node/2822965. + * * @see contextual_toolbar() */ function outside_in_toolbar_alter(&$items) { @@ -120,8 +134,7 @@ function outside_in_toolbar_alter(&$items) { // Set a class on items to mark whether they should be active in edit mode. // @todo Create a dynamic method for modules to set their own items. - // https://www.drupal.org/node/2784589 - + // https://www.drupal.org/node/2784589. $edit_mode_items = ['contextual', 'block_place']; foreach ($items as $key => $item) { if (!in_array($key, $edit_mode_items) && (!isset($items[$key]['#wrapper_attributes']['class']) || !in_array('hidden', $items[$key]['#wrapper_attributes']['class']))) { @@ -133,17 +146,54 @@ 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 a block plugin already defines its own off_canvas form, use that form + // instead of specifying one here. + if (isset($definition['forms']['off_canvas'])) { + continue; + } + + switch ($definition['id']) { + // Use specialized forms for certain blocks that do not yet provide the + // form with their own annotation. + // @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, use the block plugin's normal form rather than + // a custom form for Settings Tray. + 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 18a564c3048..a7787278153 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 ce8214697a1..5d95c1e4e50 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 00000000000..78c3167d5d1 --- /dev/null +++ b/core/modules/outside_in/src/Access/BlockPluginHasOffCanvasFormAccessCheck.php @@ -0,0 +1,48 @@ +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() + */ + 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/src/Form/OffCanvasFormAnnotationIsClassBlockForm.php b/core/modules/outside_in/tests/modules/outside_in_test/src/Form/OffCanvasFormAnnotationIsClassBlockForm.php new file mode 100644 index 00000000000..055bf878b12 --- /dev/null +++ b/core/modules/outside_in/tests/modules/outside_in_test/src/Form/OffCanvasFormAnnotationIsClassBlockForm.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 00000000000..1fd9affe9d4 --- /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 00000000000..6458fa17c6d --- /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 00000000000..1e67f234f35 --- /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 00000000000..cd0cf4838ea --- /dev/null +++ b/core/modules/outside_in/tests/src/Functional/OutsideInTest.php @@ -0,0 +1,109 @@ +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" and "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 + // \Drupal\Tests\outside_in\FunctionalJavascript\OutsideInBlockFormTest::testBlocks(). + if ($plugin_id === 'outside_in_test_false') { + $web_assert->elementNotExists('css', "{$block_selector}[data-drupal-outsidein=\"editable\"]"); + } + else { + $web_assert->elementExists('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 daf3d1f7378..0ec08cb841d 100644 --- a/core/modules/outside_in/tests/src/FunctionalJavascript/OutsideInBlockFormTest.php +++ b/core/modules/outside_in/tests/src/FunctionalJavascript/OutsideInBlockFormTest.php @@ -5,6 +5,8 @@ namespace Drupal\Tests\outside_in\FunctionalJavascript; use Drupal\block\Entity\Block; use Drupal\block_content\Entity\BlockContent; use Drupal\block_content\Entity\BlockContentType; +use Drupal\outside_in_test\Plugin\Block\OffCanvasFormAnnotationIsClassBlock; +use Drupal\outside_in_test\Plugin\Block\OffCanvasFormAnnotationNoneBlock; use Drupal\user\Entity\Role; /** @@ -36,6 +38,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', ]; /** @@ -110,6 +113,10 @@ class OutsideInBlockFormTest extends OutsideInJavascriptTestBase { // 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 +183,26 @@ class OutsideInBlockFormTest extends OutsideInJavascriptTestBase { 'button_text' => 'Save Search form', 'toolbar_item' => NULL, ], + // This is the functional JS test coverage accompanying + // \Drupal\Tests\outside_in\Functional\OutsideInTest::testPossibleAnnotations(). + OffCanvasFormAnnotationIsClassBlock::class => [ + '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 + // \Drupal\Tests\outside_in\Functional\OutsideInTest::testPossibleAnnotations(). + OffCanvasFormAnnotationNoneBlock::class => [ + '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 00000000000..09dd4eca4f6 --- /dev/null +++ b/core/modules/outside_in/tests/src/Unit/Access/BlockPluginHasOffCanvasFormAccessCheckTest.php @@ -0,0 +1,96 @@ +prophesize()->willImplement(BlockPluginInterface::class); + + if ($with_forms) { + $block_plugin->willImplement(PluginWithFormsInterface::class); + $block_plugin->hasFormClass(Argument::type('string'))->will(function ($arguments) use ($plugin_definition) { + return !empty($plugin_definition['forms'][$arguments[0]]); + }); + } + + $block = $this->prophesize(BlockInterface::class); + $block->getPlugin()->willReturn($block_plugin->reveal()); + + $access_check = new BlockPluginHasOffCanvasFormAccessCheck(); + $this->assertEquals($expected_access_result, $access_check->access($block->reveal())); + $this->assertEquals($expected_access_result, $access_check->accessBlockPlugin($block_plugin->reveal())); + } + + /** + * Provides test data for ::testAccess(). + */ + public function providerTestAccess() { + $annotation_forms_off_canvas_class = [ + 'forms' => [ + 'off_canvas' => $this->randomMachineName(), + ], + ]; + $annotation_forms_off_canvas_not_set = []; + $annotation_forms_off_canvas_false = [ + 'forms' => [ + 'off_canvas' => FALSE, + ], + ]; + return [ + 'block plugin with forms, forms[off_canvas] set to class' => [ + TRUE, + $annotation_forms_off_canvas_class, + new AccessResultAllowed(), + ], + 'block plugin with forms, forms[off_canvas] not set' => [ + TRUE, + $annotation_forms_off_canvas_not_set, + new AccessResultNeutral(), + ], + 'block plugin with forms, forms[off_canvas] set to FALSE' => [ + TRUE, + $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' => [ + FALSE, + $annotation_forms_off_canvas_class, + new AccessResultNeutral(), + ], + 'block plugin without forms, forms[off_canvas] not set' => [ + FALSE, + $annotation_forms_off_canvas_not_set, + new AccessResultNeutral(), + ], + 'block plugin without forms, forms[off_canvas] set to FALSE' => [ + FALSE, + $annotation_forms_off_canvas_false, + new AccessResultNeutral(), + ], + ]; + } + +}