Problem/Motivation

Remove non essential classes from core - the classes are in classy, cores templates should be as clean as possible.
js required classes should be prefixed to js- according to code standards

Proposed resolution

Templates to modfy

  • core/modules/system/templates/block--system-branding-block.html.twig
  • core/modules/system/templates/block--system-menu-block.html.twig
  • core/modules/system/templates/breadcrumb.html.twig

Test that no classes are clashing with js functionality, if so prefix classes with js- so its clear that the class is required by js.

Remaining tasks

User interface changes

API changes

Files: 
CommentFileSizeAuthor
#14 remove_classes_from-2407715-14.patch2.7 KBmortendk
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 86,744 pass(es). View
#11 remove_classes_from_2407715_11.patch7.84 KBManjit.Singh
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 86,242 pass(es). View
#9 remove_classes_from-2407715-9.patch2.7 KBdavidhernandez
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 86,224 pass(es). View
#5 issue-2407715-5.patch4.93 KBdernetzjaeger
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 84,114 pass(es), 26 fail(s), and 3 exception(s). View
#2 interdiff.txt5.4 KBcrowdcg
#2 2407715-2.patch3.28 KBcrowdcg
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 84,087 pass(es), 2 fail(s), and 0 exception(s). View
#1 issue-2407715-1.patch6.66 KBSivaji
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 83,952 pass(es). View

Comments

Sivaji’s picture

Status: Active » Needs review
FileSize
6.66 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 83,952 pass(es). View
crowdcg’s picture

FileSize
3.28 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 84,087 pass(es), 2 fail(s), and 0 exception(s). View
5.4 KB

Remove "set classes" and {{ attributes.addClass(classes) }} from core/modules/system/templates/block--system-menu-block.html.twig

Wondering if class="vissually-hidden" should also be removed. There is CSS for these in system so left them in for now.

Status: Needs review » Needs work

The last submitted patch, 2: 2407715-2.patch, failed testing.

davidhernandez’s picture

visually-hidden should stay. It is functional.

The test is failing because you removed the attribute from block--system-menu-block.html.twig.

+++ b/core/modules/system/templates/block--system-menu-block.html.twig
@@ -40,16 +40,8 @@
-<nav{{ attributes.addClass(classes) }} role="navigation" aria-labelledby="{{ heading_id }}">

The addClass() should be removed, but leave the attribute.

dernetzjaeger’s picture

Status: Needs work » Needs review
FileSize
4.93 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 84,114 pass(es), 26 fail(s), and 3 exception(s). View

Status: Needs review » Needs work

The last submitted patch, 5: issue-2407715-5.patch, failed testing.

davidhernandez’s picture

Status: Needs work » Postponed

Postponing this until we decide how we are moving forward. Keep an eye on #2348543: [meta] Consensus Banana Phase 2, transition templates to the Classy theme for updates. Thanks.

davidhernandez’s picture

Title: Copy system templates b*.html.twig to Classy » Remove classes from system templates b*.html.twig
Issue summary: View changes
Status: Postponed » Needs work

Un-postponing. The templates have been copied to Classy, so all we need to do is remove classes from the original templates.

davidhernandez’s picture

Status: Needs work » Needs review
FileSize
2.7 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 86,224 pass(es). View

It looks like the system menu still picks up an ID attribute from somewhere but I'm not sure where it's coming from. We may need to track that down.

mortendk’s picture

Status: Needs review » Reviewed & tested by the community

the classes site-slogan, site.name, site-logo are only used in bartik
.block-menu only in bartik

.breadcrumb is in system.theme.css - i would suggest we collect to a css followup and move specific theme elements to seven & bartik, but that is out of scope of this issue

Manjit.Singh’s picture

FileSize
7.84 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 86,242 pass(es). View

Add all changes in one file that we have discussed above.

mortendk’s picture

Issue summary: View changes

@11 template files in classy have been reorganized & have been copied over, your patch duplicate the templates in classy.
i will update the issue summary to reflect that.
patch #9 is rtbc

mortendk’s picture

Issue summary: View changes
mortendk’s picture

FileSize
2.7 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 86,744 pass(es). View

reuploads davids patch from #9 & updated the issue, to reflect the changes after the move of templates into classy

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Template changes are permitted during beta. Committed af53fb6 and pushed to 8.0.x. Thanks!

  • alexpott committed af53fb6 on 8.0.x
    Issue #2407715 by crowdcg, davidhernandez, mortendk, sivaji@knackforge....

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.