Problem/Motivation

toolbar is a beast css classes are used allover as js selectors & for visua styling it all makes it close to impossible to work with it :(
theres already 3-4 other issues that are trying to cleanup / simply toolbar, This work will be hard to get done prober if a seperation isnt done first, and as a part of the banana concensus we want to seperate css from javascript selectors.

Proposed resolution

add in prefixed .js- classes to seperate the css & js
same approach as done with forms & indentation.

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#26 interdiff-2452343-24-26.txt698 byteshimanshu-dixit
#26 2452343-26.patch12.39 KBhimanshu-dixit
#24 toolbar-2452343-23.patch12.4 KBsudhanshug
#18 toolbar-2452343-18.patch16.01 KBpektinasen
#9 toolbar_add_javascript_2452343-8.patch13.17 KBbrahmjeet789
#7 toolbar_add_javascript_2452343-7.patch23.19 KBbrahmjeet789
#6 toolbar-3.diff15.53 KBmortendk
#4 toolbar-2.diff15.92 KBmortendk
toolbar.diff16.83 KBmortendk

Comments

mortendk’s picture

mortendk commented
Issue summary: View changes
mortendk’s picture

mortendk commented
Issue summary: View changes
Related issues: +#2419135: Change the used CSS classes to follow the coding standards, +#2195695: Admin UIs on the front-end are difficult to theme, +#1995272: [Meta] Refactor module CSS files inline with our CSS standards
lewisnyman’s picture

lewisnyman
he/him
commented
Status: Needs review » Needs work 
+++ b/core/modules/toolbar/css/toolbar.module.css
@@ -231,12 +231,12 @@ body.toolbar-tray-open.toolbar-vertical.toolbar-fixed {
-.toolbar .toolbar-tray .toolbar-toggle-orientation {
+.toolbar .toolbar-tray .js-toolbar-toggle-orientation {
...
-.toolbar-oriented .toolbar-tray .toolbar-toggle-orientation {
+.toolbar-oriented .toolbar-tray .js-toolbar-toggle-orientation {

This is not correct. We shouldn't have js- classes used for styling in our CSS. We should only use use js- classes for javascript functionality.

mortendk’s picture

mortendk commented
Status: Needs work » Needs review
StatusFileSize
new toolbar-2.diff15.92 KB

removed the js- css stuff

Status: Needs review » Needs work

The last submitted patch, 4: toolbar-2.diff, failed testing.

mortendk’s picture

mortendk commented
Status: Needs work » Needs review
StatusFileSize
new toolbar-3.diff15.53 KB

rerolled

brahmjeet789’s picture

brahmjeet789 commented
StatusFileSize
new toolbar_add_javascript_2452343-7.patch23.19 KB

Status: Needs review » Needs work

The last submitted patch, 7: toolbar_add_javascript_2452343-7.patch, failed testing.

brahmjeet789’s picture

brahmjeet789 commented
StatusFileSize
new toolbar_add_javascript_2452343-8.patch13.17 KB
pjbaert’s picture

pjbaert
Primary language Dutch
commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 9: toolbar_add_javascript_2452343-8.patch, failed testing.

mathieuspil’s picture

mathieuspil commented

@brahmjeet789 I tried a interdiff of #6 and #8 and I noticed we going a total different way.

I would suggest we build a bit more on @mortendk's latest suggestion.

Patch from #6 no longer applies, so we need a reroll here.

mathieuspil’s picture

mathieuspil commented
mathieuspil’s picture

mathieuspil commented
Assigned: Unassigned » mathieuspil
mathieuspil’s picture

mathieuspil commented

@pektinasen will create this reroll.

After turning that patch green, we will merge this ticket with #2419135: Change the used CSS classes to follow the coding standards because of exactly the same code almost.

mathieuspil’s picture

mathieuspil commented
Assigned: mathieuspil » Unassigned
pektinasen’s picture

pektinasen commented
Assigned: Unassigned » pektinasen
pektinasen’s picture

pektinasen commented
StatusFileSize
new toolbar-2452343-18.patch16.01 KB

I rerolled #2452343-6: toolbar add in javascript classes to remove confusion with css but it breaks the layout.

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.

sudhanshug’s picture

sudhanshug commented
Status: Needs work » Needs review

Changing status to test the patch.

Status: Needs review » Needs work

The last submitted patch, 18: toolbar-2452343-18.patch, failed testing.

sudhanshug’s picture

sudhanshug commented
Status: Needs work » Needs review
StatusFileSize
new toolbar-2452343-23.patch12.4 KB

Re-rolled the patch from 18 to apply cleanly to the latest D8

Status: Needs review » Needs work

The last submitted patch, 24: toolbar-2452343-23.patch, failed testing.

himanshu-dixit’s picture

himanshu-dixit commented
Status: Needs work » Needs review
StatusFileSize
new 2452343-26.patch12.39 KB
new interdiff-2452343-24-26.txt698 bytes

Looks like testbot doesn't agree to the last patch. So, I have removed quotes from the property since it is redundant according to coding standards.

Status: Needs review » Needs work

The last submitted patch, 26: 2452343-26.patch, failed testing.

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.

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.

smustgrave’s picture

smustgrave commented
Status: Needs work » Postponed (maintainer needs more info)
Issue tags: -JavaScript +JavaScript, +stale-issue-cleanup

Thank you for reporting this problem. We rely on issue reports like this one to resolve bugs and improve Drupal core.

Since there has been no activity here for over 8 years we are asking if this problem persists on a currently supported version of Drupal. To help, add a comment explaining if the problem still occurs or not. Any extra detail you can provide can help others who experienced this.

Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.
Thanks!

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

smustgrave’s picture

smustgrave commented
Project: Drupal core » Toolbar
Version: main » 1.x-dev
Component: CSS » Code

Since toolbar is being deprecated in D12 going to move this over to the contrib space