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
simpletest should be done with classy theme, not stark

Proposed resolution

Twig Templates to modify

  • core/modules/system/templates/page.html.twig
  • core/modules/system/templates/pager.html.twig
  • core/modules/system/templates/progress-bar.html.twig

Remaining tasks

User interface changes

See parent issue(s) and related issue(s) for details,

CommentFileSizeAuthor
#21 p-templates-5.diff10.3 KBvalderama
#20 p-templates-4.diff9.61 KBvalderama
#16 p-templates-3.diff9.01 KBmortendk
#11 progressbar-before.png42.52 KBrteijeiro
#11 progressbar-after.png43.58 KBrteijeiro
#10 p-templates-2.diff9.57 KBmortendk
#10 Installing_Drupal___Drupal.png509.67 KBmortendk
#4 clean-p-templates.diff6.04 KBmortendk
#1 issue-2407737-1.patch16.72 KBsivaji_ganesh_jojodae

Comments

sivaji_ganesh_jojodae’s picture

sivaji_ganesh_jojodae commented
Status: Active » Needs review
StatusFileSize
new issue-2407737-1.patch16.72 KB
davidhernandez’s picture

davidhernandez
Primary language English
Location USA
commented
Status: Needs review » 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 starterkit theme for updates. Thanks.

davidhernandez’s picture

davidhernandez
Primary language English
Location USA
commented
Title: Copy system templates p*.html.twig to Classy » Remove classes from system templates p*.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.

mortendk’s picture

mortendk commented
Status: Needs work » Needs review
StatusFileSize
new clean-p-templates.diff6.04 KB

progress-bar.html.twig have a lot of tie ins with core/misc/progress.js
must be looked into renaming classes there to js- progress__label etc

pager.html.twig & page.html.twig should be clean

mortendk’s picture

mortendk commented
Issue summary: View changes
Issue tags: +Classy
mortendk’s picture

mortendk commented
Status: Needs review » Needs work
Issue tags: +drupalcafe-feb2015

pregressbar css needs to be renamed & tested

xen’s picture

xen commented
Assigned: Unassigned » xen

Grabbing.

xen’s picture

xen commented
Assigned: xen » Unassigned

Arg.. Ungrabbing.

mortendk’s picture

mortendk commented
Issue tags: -drupalcafe-feb2015 +Needs screenshots
mortendk’s picture

mortendk commented
Issue summary: View changes
Status: Needs work » Needs review
StatusFileSize
new Installing_Drupal___Drupal.png509.67 KB
new p-templates-2.diff9.57 KB

prefixed the js needed classes with .js- and took screenshots

rteijeiro’s picture

rteijeiro commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new progressbar-after.png43.58 KB
new progressbar-before.png42.52 KB

Great job @mortendk!

Progressbar BEFORE

Progressbar AFTER

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs review

Something is feeling really wrong about prefixing everything that javascript touches with js- especially when the class is being used to give the element style.

davidhernandez’s picture

davidhernandez
Primary language English
Location USA
commented
Status: Needs review » Needs work

Agreeing with Alex. I don't think js- classes should show up in CSS files, and most of the changes in this patch are CSS. The point of the prefix is to highlight classes that are only needed for javascript. If we use them for both, it is no different than using the non-js classes for style and javascript. I'm thinking add separate js- classes or rework the javascript.

mortendk’s picture

mortendk commented

yeah looks like we also have to rewrite all of that
Well now at least we know where this body is burried

mortendk’s picture

mortendk commented
Issue tags: -Classy, -Needs screenshots +jsbanana
mortendk’s picture

mortendk commented
Status: Needs work » Needs review
StatusFileSize
new p-templates-3.diff9.01 KB

ok i have taken same approach with the js- prefix as in forms & indentation etc.
js- prefix classes added in for js operations

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Issue tags: +Needs change record updates

We need to update the CR

mortendk’s picture

mortendk commented
Issue tags: +Needs screenshots

updated with the changes in the css classes, whats added in as js- prefixes & which classes we have removed in core, but is in classy

Do we want to fix the css MAT seperation here as well - and screenshots since we are changing js selectors ?

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Needs work 
  1. +++ b/core/misc/progress.js
@@ -21,10 +21,10 @@
+    this.element.html('<div class="js-progress__label progress__label">&nbsp;</div>' +
+      '<div class="progress__track"><div class="js-progress__bar progress__bar"></div></div>' +
+      '<div class="js-progress__percentage progress__percentage"></div>' +
+      '<div class="js-progress__description progress__description">&nbsp;</div>');
   };

    Maybe it would be nice to do #2452381: Use Drupal.theme for progress.js 

  2. +++ b/core/modules/system/templates/pager.html.twig
@@ -32,12 +32,12 @@
-  <nav class="pager" role="navigation" aria-labelledby="pagination-heading">
-    <h4 id="pagination-heading" class="visually-hidden">{{ 'Pagination'|t }}</h4>
-    <ul class="pager__items">
+  <nav role="navigation" aria-labelledby="pagination-heading">
+    <h4 class="visually-hidden">{{ 'Pagination'|t }}</h4>
+    <ul>

    So, I see 

        this.$view.find('ul.pager__items > li > a, th.views-field a, .attachment .views-summary a')
      .each(jQuery.proxy(this.attachPagerLinkAjax, this));

    in core/modules/views/js/ajax_view.js:105 ... doesn't this break things?

valderama’s picture

valderama commented
StatusFileSize
new p-templates-4.diff9.61 KB

Hi - found a reference to this issue on twitter, and thought I could do a quick patch.

I have taken on dawehners second point from the last comment and added a class "js-pager__items" to the ul-tag - and changed the jQuery selector in ajax_views.js accordingly. That should keep this part working. Everything else is the same as in the last patch.

And a little side note: Keeping the empty div here seems strong. Maybe a more semantic tag would be better? It is not a big deal however...

+++ b/core/modules/system/templates/page.html.twig
@@ -70,21 +70,21 @@
     {% if site_name or site_slogan %}
-      <div class="name-and-slogan">
+      <div>
valderama’s picture

valderama commented
Status: Needs work » Needs review
StatusFileSize
new p-templates-5.diff10.3 KB

I forgot to add the "js-pager__items" class also to the classy version of pager.html.twig. Included in the new patch.

I have tested the ajax pager now in bartik, seven and stark - and it works now in classy-themes (bartik and seven) and in non-classy themes (stark).

Status: Needs review » Needs work

The last submitted patch, 21: p-templates-5.diff, failed testing.

valderama’s picture

valderama commented

I guess the PagerTest fails because it looks for li.pager__items - which are not present in non-classy themes. Same for AggregatorRenderingTest.

So the patch fails because simpletest uses a non-classy theme?

mortendk’s picture

mortendk commented

bumb

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

quietone’s picture

quietone commented
Issue tags: -, -Needs change record updates +Needs change record

Version: 9.5.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
Status: Needs work » Closed (outdated)

This was filed ~10 months before Drupal 8 was released, any time during those 10 months would have been a fine time to take care of this.
Drupal 8+ has now been around for 8.5 years, and removing the classes now would be incredibly disruptive and cause many regressions. The intentions here were good, but any benefits they'd have yeilded are trival compared to the problems that would be introduced. Closing so nobody winds up devoting any more time to something that I (Frontend Framework Manager) would not approve.