From e3221b6ddc314669f57d5dc2dfc3aee5388fc56d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A1bor=20Hojtsy?= Date: Tue, 18 Feb 2020 18:20:22 +0100 Subject: [PATCH] Issue #3096349 by bnjmnm, dww, lauriii: Create test for confirming Themes do not depend on Classy templates (cherry picked from commit 9a5df1707c336b844e7bc3cb3071ab7c4a81f079) --- .../content/node--article--full.html.twig | 2 +- .../content/node--card-common.html.twig | 2 +- .../templates/content/node--card.html.twig | 2 +- .../umami/templates/content/node.html.twig | 2 +- .../ThemesNotUsingClassyTemplatesTest.php | 515 ++++++++++++++++++ .../block--search-form-block.html.twig | 45 +- .../block--system-menu-block.html.twig | 69 ++- core/themes/bartik/templates/node.html.twig | 2 +- .../bartik/templates/page-title.html.twig | 14 +- .../templates/status-messages.html.twig | 50 +- .../claro/templates/field/file-link.html.twig | 2 +- .../templates/form-element-label.html.twig | 22 +- .../seven/templates/image-widget.html.twig | 19 +- 13 files changed, 700 insertions(+), 46 deletions(-) create mode 100644 core/tests/Drupal/KernelTests/Core/Theme/ThemesNotUsingClassyTemplatesTest.php diff --git a/core/profiles/demo_umami/themes/umami/templates/content/node--article--full.html.twig b/core/profiles/demo_umami/themes/umami/templates/content/node--article--full.html.twig index 7cd0fe0f53c..55ae577b323 100644 --- a/core/profiles/demo_umami/themes/umami/templates/content/node--article--full.html.twig +++ b/core/profiles/demo_umami/themes/umami/templates/content/node--article--full.html.twig @@ -77,7 +77,7 @@ ] %} {% set created_date = node.getCreatedTime|format_date('umami_dates') %} -{{ attach_library('classy/node') }} +{{ attach_library('umami/classy.node') }} {{ title_prefix }} diff --git a/core/profiles/demo_umami/themes/umami/templates/content/node--card-common.html.twig b/core/profiles/demo_umami/themes/umami/templates/content/node--card-common.html.twig index 7def0d41d31..00ccfb68b5e 100644 --- a/core/profiles/demo_umami/themes/umami/templates/content/node--card-common.html.twig +++ b/core/profiles/demo_umami/themes/umami/templates/content/node--card-common.html.twig @@ -77,7 +77,7 @@ view_mode ? 'node--view-mode-' ~ view_mode|clean_class, ] %} -{{ attach_library('classy/node') }} +{{ attach_library('umami/classy.node') }} {{ attach_library('umami/view-mode-card') }} {% block libraries %} {{ attach_library('umami/view-mode-card-common') }} diff --git a/core/profiles/demo_umami/themes/umami/templates/content/node--card.html.twig b/core/profiles/demo_umami/themes/umami/templates/content/node--card.html.twig index 8a127561549..9835c2e0fe2 100644 --- a/core/profiles/demo_umami/themes/umami/templates/content/node--card.html.twig +++ b/core/profiles/demo_umami/themes/umami/templates/content/node--card.html.twig @@ -77,7 +77,7 @@ view_mode ? 'node--view-mode-' ~ view_mode|clean_class, ] %} -{{ attach_library('classy/node') }} +{{ attach_library('umami/classy.node') }} {{ attach_library('umami/view-mode-card') }} diff --git a/core/profiles/demo_umami/themes/umami/templates/content/node.html.twig b/core/profiles/demo_umami/themes/umami/templates/content/node.html.twig index 359f926dd1f..64122a30249 100644 --- a/core/profiles/demo_umami/themes/umami/templates/content/node.html.twig +++ b/core/profiles/demo_umami/themes/umami/templates/content/node.html.twig @@ -77,7 +77,7 @@ ] %} {% set created_date = node.getCreatedTime|format_date('umami_dates') %} -{{ attach_library('classy/node') }} +{{ attach_library('umami/classy.node') }} diff --git a/core/tests/Drupal/KernelTests/Core/Theme/ThemesNotUsingClassyTemplatesTest.php b/core/tests/Drupal/KernelTests/Core/Theme/ThemesNotUsingClassyTemplatesTest.php new file mode 100644 index 00000000000..e593bc577ef --- /dev/null +++ b/core/tests/Drupal/KernelTests/Core/Theme/ThemesNotUsingClassyTemplatesTest.php @@ -0,0 +1,515 @@ +themeHandler = $this->container->get('theme_handler'); + $this->container->get('theme_installer')->install([ + 'umami', + 'bartik', + 'seven', + 'claro', + ]); + // Enable all modules so every template is present in the theme registry. + // This makes it possible to check the source of every template and + // determine if they come from Classy. + $this->installAllModules(); + } + + /** + * Installs all core modules. + */ + protected function installAllModules() { + // Enable all core modules. + $all_modules = $this->container->get('extension.list.module')->getList(); + $all_modules = array_filter($all_modules, function ($module) { + // Filter contrib, hidden, experimental, already enabled modules, and + // modules in the Testing package. + if ($module->origin !== 'core' || !empty($module->info['hidden']) || $module->status === TRUE || $module->info['package'] === 'Testing' || $module->info['package'] === 'Core (Experimental)') { + return FALSE; + } + return TRUE; + }); + $all_modules = array_keys($all_modules); + $module_installer = $this->container->get('module_installer'); + $module_installer->install($all_modules); + } + + /** + * Ensures that themes are not inheriting templates from Classy. + * + * @param string $theme + * The theme to test. + * @param string[] $templates_to_skip + * Templates that will not be tested. + * + * @dataProvider providerTestThemesTemplatesNotClassy + */ + public function testThemesTemplatesNotClassy($theme, array $templates_to_skip) { + // Get every template available to the theme being tested. + $theme_registry = new Registry($this->root, \Drupal::cache(), \Drupal::lock(), \Drupal::moduleHandler(), $this->themeHandler, \Drupal::service('theme.initialization'), $theme); + $theme_registry->setThemeManager(\Drupal::theme()); + $theme_registry_full = $theme_registry->get(); + + // Loop through every template available to the current theme, confirm it + // does not come from Classy, does not attach Classy libraries, and does not + // extend or include Classy templates. + foreach ($theme_registry_full as $info) { + if (isset($info['template'])) { + $template_name = $info['template']; + + if (in_array($template_name, $templates_to_skip) || in_array($template_name, $this->templatesSkippableBecauseIdenticalToStable)) { + continue; + } + + $template_contents = file_get_contents("{$this->root}/{$info['path']}/$template_name.html.twig"); + + // Confirm template does not come from Classy. + $this->assertFalse($info['theme path'] === 'core/themes/classy', "$theme is inheriting $template_name from Classy."); + + // Confirm template does not include or extend Classy templates. + preg_match_all('/(extends|include)\s+(\'|")@classy/', $template_contents, $classy_extend_include_matches); + $this->assertEmpty($classy_extend_include_matches[0], "The template: '$template_name' in the theme: '$theme' includes or extends a Classy template."); + + // Confirm template does not attach a Classy library. + preg_match_all('/attach_library\((\'|")classy\/.+(\'|")\)/', $template_contents, $classy_extend_library_matches); + $this->assertEmpty($classy_extend_library_matches[0], "The template: '$template_name' in the theme: '$theme' attaches a Classy library."); + } + } + } + + /** + * Data provider for testThemesTemplatesNotClassy(). + * + * @return array + * Array of test cases using these keys: + * -'theme-name': The machine name of the theme being tested. + * -'to-skip': Templates that will skipped by the test. + */ + public function providerTestThemesTemplatesNotClassy() { + // Each item provides the theme name and an array of templates to skip. The + // templates in the to-skip array are ones that have not yet been decoupled + // from Classy. When a template is properly decoupled from Classy, it can be + // removed from to-skip. If this test passes with an empty to-skip array, + // this is confirmation that the templates are fully decoupled form Classy. + return [ + 'umami' => [ + 'theme-name' => 'umami', + 'to-skip' => [ + 'aggregator-feed', + 'aggregator-item', + 'block', + 'book-export-html', + 'book-all-books-block', + 'book-tree', + 'book-navigation', + 'book-node-export-html', + 'comment', + 'field--comment', + 'file-managed-file', + 'file-audio', + 'file-link', + 'file-video', + 'filter-guidelines', + 'filter-tips', + 'text-format-wrapper', + 'filter-caption', + 'form-element-label', + 'forum-list', + 'forum-icon', + 'forums', + 'forum-submitted', + 'help-section', + 'image-widget', + 'link-formatter-link-separate', + 'media-embed-error', + 'media', + 'media--media-library', + 'media-library-wrapper', + 'media-library-item', + 'views-view-unformatted--media-library', + 'item-list--search-results', + 'links--node', + 'field--text', + 'field--text-long', + 'field--text-with-summary', + 'links--media-library-menu', + 'container--media-library-content', + 'media-library-item--small', + 'container--media-library-widget-selection', + 'block--local-tasks-block', + 'block--search-form-block', + 'node-edit-form', + 'node-add-list', + 'field--node--uid', + 'field--node--title', + 'field--node--created', + 'rdf-metadata', + 'search-result', + 'progress-bar', + 'datetime-wrapper', + 'fieldset', + 'datetime-form', + 'textarea', + 'details', + 'form-element', + 'radios', + 'item-list', + 'table', + 'maintenance-page', + 'html', + 'region', + 'breadcrumb', + 'menu-local-tasks', + 'page-title', + 'mark', + 'time', + 'image', + 'field', + 'block--local-actions-block', + 'block--system-menu-block', + 'taxonomy-term', + 'toolbar', + 'username', + 'user', + 'views-form-views-form', + 'views-mini-pager', + 'views-exposed-form', + 'views-view-grouping', + 'views-view-summary', + 'views-view-table', + 'views-view-row-rss', + 'views-view-summary-unformatted', + 'views-view', + ], + ], + 'seven' => [ + 'theme-name' => 'seven', + 'to-skip' => [ + 'aggregator-feed', + 'aggregator-item', + 'block--system-branding-block', + 'block', + 'breadcrumb', + 'book-export-html', + 'book-all-books-block', + 'book-tree', + 'book-navigation', + 'book-node-export-html', + 'comment', + 'field--comment', + 'file-managed-file', + 'file-audio', + 'file-link', + 'file-video', + 'filter-guidelines', + 'filter-tips', + 'text-format-wrapper', + 'filter-caption', + 'form-element-label', + 'forum-list', + 'forum-icon', + 'forums', + 'forum-submitted', + 'help-section', + 'link-formatter-link-separate', + 'media-embed-error', + 'media', + 'media--media-library', + 'media-library-wrapper', + 'media-library-item', + 'views-view-unformatted--media-library', + 'item-list--search-results', + 'links--node', + 'field--text', + 'field--text-long', + 'field--text-with-summary', + 'links--media-library-menu', + 'container--media-library-content', + 'media-library-item--small', + 'container--media-library-widget-selection', + 'block--local-tasks-block', + 'block--search-form-block', + 'node', + 'field--node--uid', + 'field--node--title', + 'field--node--created', + 'rdf-metadata', + 'search-result', + 'progress-bar', + 'status-messages', + 'datetime-wrapper', + 'fieldset', + 'datetime-form', + 'textarea', + 'form-element', + 'radios', + 'item-list', + 'table', + 'html', + 'region', + 'menu', + 'menu-local-task', + 'page-title', + 'mark', + 'time', + 'image', + 'field', + 'block--system-menu-block', + 'taxonomy-term', + 'toolbar', + 'username', + 'user', + 'views-form-views-form', + 'views-mini-pager', + 'views-exposed-form', + 'views-view-grouping', + 'views-view-summary', + 'views-view-table', + 'views-view-row-rss', + 'views-view-summary-unformatted', + 'views-view', + ], + ], + 'claro' => [ + 'theme-name' => 'claro', + 'to-skip' => [ + 'aggregator-feed', + 'aggregator-item', + 'block--system-branding-block', + 'block', + 'book-export-html', + 'book-all-books-block', + 'book-tree', + 'book-navigation', + 'book-node-export-html', + 'comment', + 'field--comment', + 'file-audio', + 'file-video', + 'filter-caption', + 'forum-list', + 'forum-icon', + 'forums', + 'forum-submitted', + 'help-section', + 'link-formatter-link-separate', + 'media-embed-error', + 'media', + 'media--media-library', + 'media-library-wrapper', + 'media-library-item', + 'views-view-unformatted--media-library', + 'item-list--search-results', + 'links--node', + 'field--text', + 'field--text-long', + 'field--text-with-summary', + 'links--media-library-menu', + 'container--media-library-content', + 'media-library-item--small', + 'container--media-library-widget-selection', + 'block--local-tasks-block', + 'block--search-form-block', + 'node', + 'field--node--uid', + 'field--node--title', + 'field--node--created', + 'rdf-metadata', + 'search-result', + 'progress-bar', + 'textarea', + 'radios', + 'item-list', + 'table', + 'html', + 'region', + 'menu', + 'page-title', + 'mark', + 'time', + 'image', + 'field', + 'block--local-actions-block', + 'block--system-menu-block', + 'taxonomy-term', + 'toolbar', + 'username', + 'user', + 'views-form-views-form', + 'views-view-grouping', + 'views-view-summary', + 'views-view-table', + 'views-view-row-rss', + 'views-view-summary-unformatted', + 'views-view', + ], + ], + 'bartik' => [ + 'theme-name' => 'bartik', + 'to-skip' => [ + 'aggregator-feed', + 'aggregator-item', + 'block--system-branding-block', + 'block', + 'book-export-html', + 'book-all-books-block', + 'book-tree', + 'book-navigation', + 'book-node-export-html', + 'comment', + 'field--comment', + 'file-managed-file', + 'file-audio', + 'file-link', + 'file-video', + 'filter-guidelines', + 'filter-tips', + 'text-format-wrapper', + 'filter-caption', + 'forum-list', + 'forum-icon', + 'forums', + 'forum-submitted', + 'help-section', + 'image-widget', + 'link-formatter-link-separate', + 'media-embed-error', + 'media', + 'media--media-library', + 'media-library-wrapper', + 'media-library-item', + 'views-view-unformatted--media-library', + 'item-list--search-results', + 'links--node', + 'field--text', + 'field--text-long', + 'field--text-with-summary', + 'links--media-library-menu', + 'container--media-library-content', + 'media-library-item--small', + 'container--media-library-widget-selection', + 'block--local-tasks-block', + 'node-edit-form', + 'node-add-list', + 'field--node--uid', + 'field--node--title', + 'field--node--created', + 'rdf-metadata', + 'search-result', + 'progress-bar', + 'datetime-wrapper', + 'fieldset', + 'datetime-form', + 'textarea', + 'details', + 'form-element', + 'form-element-label', + 'radios', + 'item-list', + 'table', + 'page', + 'maintenance-page', + 'html', + 'region', + 'menu', + 'menu-local-task', + 'breadcrumb', + 'menu-local-tasks', + 'mark', + 'time', + 'image', + 'field', + 'block--local-actions-block', + 'taxonomy-term', + 'toolbar', + 'username', + 'user', + 'views-mini-pager', + 'views-exposed-form', + 'views-form-views-form', + 'views-view-grouping', + 'views-view-summary', + 'views-view-table', + 'views-view-row-rss', + 'views-view-summary-unformatted', + 'views-view', + ], + ], + ]; + } + +} diff --git a/core/themes/bartik/templates/block--search-form-block.html.twig b/core/themes/bartik/templates/block--search-form-block.html.twig index b2af1edf2b4..1f9adf0d326 100644 --- a/core/themes/bartik/templates/block--search-form-block.html.twig +++ b/core/themes/bartik/templates/block--search-form-block.html.twig @@ -1,23 +1,48 @@ -{% extends "@classy/block/block--search-form-block.html.twig" %} {# /** * @file - * Bartik's theme implementation for a search form block. Extends Classy's - * search form block template. + * Bartik's theme implementation for a search form block. * * Available variables: + * - plugin_id: The ID of the block implementation. + * - label: The configured label of the block if visible. + * - configuration: A list of the block's configuration values, including: + * - label: The configured label for the block. + * - label_display: The display settings for the label. + * - provider: The module or other provider that provided this block plugin. + * - Block plugin specific settings will also be stored here. * - content: The content of this block. * - content_attributes: A list of HTML attributes applied to the main content + * - attributes: A list HTML attributes populated by modules, intended to + * be added to the main container tag of this template. Includes: + * - id: A valid HTML ID and guaranteed unique. + * - title_attributes: Same as attributes, except applied to the main title * tag that appears in the template. + * - title_prefix: Additional output populated by modules, intended to be + * displayed in front of the main title tag that appears in the template. + * - title_suffix: Additional output populated by modules, intended to be + * displayed after the main title tag that appears in the template. * * @see template_preprocess_block() * @see search_preprocess_block() - * - * @ingroup themeable */ #} -{% block content %} - - {{ parent() }} - -{% endblock %} +{% + set classes = [ + 'block', + 'block-search', + 'container-inline', + ] +%} + + {{ title_prefix }} + {% if label %} + {{ label }} + {% endif %} + {{ title_suffix }} + {% block content %} + + {{ content }} + + {% endblock %} + diff --git a/core/themes/bartik/templates/block--system-menu-block.html.twig b/core/themes/bartik/templates/block--system-menu-block.html.twig index d22cfbf9316..aea280820ef 100644 --- a/core/themes/bartik/templates/block--system-menu-block.html.twig +++ b/core/themes/bartik/templates/block--system-menu-block.html.twig @@ -1,21 +1,66 @@ -{% extends "@classy/block/block--system-menu-block.html.twig" %} {# /** * @file * Bartik's theme implementation for a menu block. * - * @ingroup themeable + * Available variables: + * - plugin_id: The ID of the block implementation. + * - label: The configured label of the block if visible. + * - configuration: A list of the block's configuration values. + * - label: The configured label for the block. + * - label_display: The display settings for the label. + * - provider: The module or other provider that provided this block plugin. + * - Block plugin specific settings will also be stored here. + * - content: The content of this block. + * - attributes: HTML attributes for the containing element. + * - id: A valid HTML ID and guaranteed unique. + * - title_attributes: HTML attributes for the title element. + * - content_attributes: HTML attributes for the content element. + * - title_prefix: Additional output populated by modules, intended to be + * displayed in front of the main title tag that appears in the template. + * - title_suffix: Additional output populated by modules, intended to be + * displayed after the main title tag that appears in the template. + * + * Headings should be used on navigation menus that consistently appear on + * multiple pages. When this menu block's label is configured to not be + * displayed, it is automatically made invisible using the 'visually-hidden' CSS + * class, which still keeps it visible for screen-readers and assistive + * technology. Headings allow screen-reader and keyboard only users to navigate + * to or skip the links. + * See http://juicystudio.com/article/screen-readers-display-none.php and + * http://www.w3.org/TR/WCAG-TECHS/H42.html for more information. */ #} +{% + set classes = [ + 'block', + 'block-menu', + 'navigation', + 'menu--' ~ derivative_plugin_id|clean_class, + ] +%} +{% set heading_id = attributes.id ~ '-menu'|clean_id %} {% set show_anchor = "show-" ~ attributes.id|clean_id %} {% set hide_anchor = "hide-" ~ attributes.id|clean_id %} -{% block content %} - - {# When rendering a menu without label, render a menu toggle. #} - - - {% trans %} Show — {{ configuration.label }}{% endtrans %} - {% trans %} Hide — {{ configuration.label }}{% endtrans %} - {{ content }} - -{% endblock %} + + diff --git a/core/themes/bartik/templates/node.html.twig b/core/themes/bartik/templates/node.html.twig index c7495223fed..c0df4837abb 100644 --- a/core/themes/bartik/templates/node.html.twig +++ b/core/themes/bartik/templates/node.html.twig @@ -77,7 +77,7 @@ 'clearfix', ] %} -{{ attach_library('classy/node') }} +{{ attach_library('claro/classy.node') }}
{{ title_prefix }} diff --git a/core/themes/bartik/templates/page-title.html.twig b/core/themes/bartik/templates/page-title.html.twig index e061cd2e019..4d12311f87f 100644 --- a/core/themes/bartik/templates/page-title.html.twig +++ b/core/themes/bartik/templates/page-title.html.twig @@ -1,4 +1,3 @@ -{% extends "@classy/content/page-title.html.twig" %} {# /** * @file @@ -13,4 +12,15 @@ * displayed after the main title tag that appears in the template. */ #} -{% set title_attributes = title_attributes.addClass('title') %} +{% + set classes = [ + 'title', + 'page-title', + ] +%} + +{{ title_prefix }} +{% if title %} + {{ title }} +{% endif %} +{{ title_suffix }} diff --git a/core/themes/bartik/templates/status-messages.html.twig b/core/themes/bartik/templates/status-messages.html.twig index e822ff5fbb9..700593206e0 100644 --- a/core/themes/bartik/templates/status-messages.html.twig +++ b/core/themes/bartik/templates/status-messages.html.twig @@ -1,4 +1,3 @@ -{% extends "@classy/misc/status-messages.html.twig" %} {# /** * @file @@ -16,13 +15,46 @@ * Available variables: * - message_list: List of messages to be displayed, grouped by type. * - status_headings: List of all status types. + * - attributes: HTML attributes for the element, including: + * - class: HTML classes. */ #} -{% block messages %} - {% if message_list is not empty %} - {{ attach_library('bartik/messages') }} -
- {{ parent() }} -
- {% endif %} -{% endblock messages %} +
+ {% block messages %} + {% if message_list is not empty %} + {{ attach_library('bartik/messages') }} +
+ {% for type, messages in message_list %} + {% + set classes = [ + 'messages', + 'messages--' ~ type, + ] + %} +
+ {% if type == 'error' %} +
+ {% endif %} + {% if status_headings[type] %} +

{{ status_headings[type] }}

+ {% endif %} + {% if messages|length > 1 %} +
    + {% for message in messages %} +
  • {{ message }}
  • + {% endfor %} +
+ {% else %} + {{ messages|first }} + {% endif %} + {% if type == 'error' %} +
+ {% endif %} +
+ {# Remove type specific classes. #} + {% set attributes = attributes.removeClass(classes) %} + {% endfor %} +
+ {% endif %} + {% endblock messages %} +
diff --git a/core/themes/claro/templates/field/file-link.html.twig b/core/themes/claro/templates/field/file-link.html.twig index 891c7a124d3..07cd9e8561d 100644 --- a/core/themes/claro/templates/field/file-link.html.twig +++ b/core/themes/claro/templates/field/file-link.html.twig @@ -14,5 +14,5 @@ * @see stable_preprocess_image_widget() */ #} -{{ attach_library('classy/file') }} +{{ attach_library('claro/classy.file') }} {{ link }} ({{ file_size }}) diff --git a/core/themes/claro/templates/form-element-label.html.twig b/core/themes/claro/templates/form-element-label.html.twig index f05010d686d..d0ec8a3b1a0 100644 --- a/core/themes/claro/templates/form-element-label.html.twig +++ b/core/themes/claro/templates/form-element-label.html.twig @@ -2,11 +2,25 @@ /** * @file * Template override for a form element label. + * + * Available variables: + * - title: The label's text. + * - title_display: Elements title_display setting. + * - required: An indicator for whether the associated form element is required. + * - attributes: A list of HTML attributes for the label. + * + * @see template_preprocess_form_element_label() */ #} - -{% extends '@classy/form/form-element-label.html.twig' %} - {% - set attributes = attributes.addClass('form-item__label') + set classes = [ + 'form-item__label', + title_display == 'after' ? 'option', + title_display == 'invisible' ? 'visually-hidden', + required ? 'js-form-required', + required ? 'form-required', + ] %} +{% if title is not empty or required -%} + {{ title }} +{%- endif %} diff --git a/core/themes/seven/templates/image-widget.html.twig b/core/themes/seven/templates/image-widget.html.twig index a7cf3ad48a1..2d6f4014ed5 100644 --- a/core/themes/seven/templates/image-widget.html.twig +++ b/core/themes/seven/templates/image-widget.html.twig @@ -3,10 +3,23 @@ * @file * Theme override for an image field widget. * - * Included from Classy theme. + * Available variables: + * - attributes: HTML attributes for the containing element. + * - data: Render elements of the image widget. * * @see template_preprocess_image_widget() */ #} -{% include '@classy/content-edit/image-widget.html.twig' %} -{{ attach_library('classy/image-widget') }} +{{ attach_library('seven/classy.image-widget') }} + + + {% if data.preview %} +
+ {{ data.preview }} +
+ {% endif %} +
+ {# Render widget data without the image preview that was output already. #} + {{ data|without('preview') }} +
+