Refactor Claro's dropbutton stylesheet

The Needs Review Queue Bot → tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

More nesting can be done for

.dropbutton__toggle
.js.no-touchevents

large file but a lot of those can be folded into each other.

Opened a new MR using @Gauravvvv and @starshaped's cherrypicked commits from !3547 to untangle a botched rebase. Going forward, as long as this issue doesn't get bumped to 10.2.x, contributors should be fine to merge origin/10.1.x back into this MR branch.

Thanks @andy-blum!

@smustgrave, I actually wasn't quite done with my refactoring and still need to fix a few issues, so I'm moving this back to needs work. Thanks @andy-blum for fixing my mess, I'll work off that new MR!

Also #17 is not in any way a review - just getting things back on track.

Hi @starshaped
In the last commit (7a272525 - Cleaned up CSS) you removed logical property and wrote normal css in place of that.
But I think this is wrong approach as in the Proposed resolution they asked to Use CSS Logical Properties where appropriate. :)

@Santosh_Verma Yep, this was on purpose because the styles would not display correctly in RTL mode when using logical properties. Since using logical properties here caused a regression in the layout, I used non-logical properties instead.

Also, unassigning myself to this issue as I haven't had time to work on it--anyone, feel free to pick this one up!

This has changed a few times. Could we get before/after screenshots please. Also screenshots of rtl please.

Thanks.

I have tested the #MR3615

there is error in the patch

Checking patch core/themes/claro/css/components/dropbutton.pcss.css...
error: while searching for:
  overflow: visible;
  margin: 0;
  list-style: none;
}
[dir="rtl"] .dropbutton {
  margin: 0;
}

.js .dropbutton {
  min-width: 6rem; /* Help mitigate long dropbutton text from obscuring other dropbuttons. */
  height: var(--dropbutton-toggle-size);
}

/* Variants. */
.js.no-touchevents .dropbutton--small {
  height: var(--dropbutton-small-toggle-size);
}

.js.no-touchevents .dropbutton--extrasmall {
  height: var(--dropbutton-extrasmall-toggle-size);
}

/**
 * First dropbutton list item.
 */
.js .dropbutton--multiple .dropbutton__item:first-of-type {
  margin-right: calc(var(--dropbutton-toggle-size) + var(--dropbutton-toggle-size-spacing)); /* LTR */
}
[dir="rtl"].js .dropbutton--multiple .dropbutton__item:first-of-type {
  margin-right: 0;
  margin-left: calc(var(--dropbutton-toggle-size) + var(--dropbutton-toggle-size-spacing));
}

/* First dropbutton list item variants */
.js.no-touchevents .dropbutton--multiple.dropbutton--small .dropbutton__item:first-of-type {
  margin-right: calc(var(--dropbutton-small-toggle-size) + var(--dropbutton-toggle-size-spacing)); /* LTR */
}
[dir="rtl"].js.no-touchevents .dropbutton--multiple.dropbutton--small .dropbutton__item:first-of-type {
  margin-right: 0;
  margin-left: calc(var(--dropbutton-small-toggle-size) + var(--dropbutton-toggle-size-spacing));
}

.js.no-touchevents .dropbutton--multiple.dropbutton--extrasmall .dropbutton__item:first-of-type {
  margin-right: calc(var(--dropbutton-extrasmall-toggle-size) + var(--dropbutton-toggle-size-spacing)); /* LTR */
}
[dir="rtl"].js.no-touchevents .dropbutton--multiple.dropbutton--extrasmall .dropbutton__item:first-of-type {
  margin-right: 0;
  margin-left: calc(var(--dropbutton-extrasmall-toggle-size) + var(--dropbutton-toggle-size-spacing));
}

/**

error: patch failed: core/themes/claro/css/components/dropbutton.pcss.css:62
error: core/themes/claro/css/components/dropbutton.pcss.css: patch does not apply

Try to attach screenshot too Stan

Patch LGTM !

patch doesn't apply to 11.x

Patch for 11.x.

Can't apply patch from #30, so did reroll again of the patch from #27 against 11.x

+++ b/core/themes/claro/css/components/dropbutton.pcss.css
@@ -250,63 +236,63 @@
+  & > *:active {

The nesting is not right here. Resulting selector is
.dropbutton__item:first-of-type > * > *:active Where it should be

.dropbutton__item:first-of-type > *:active {

Fixed CCF in #33.

There are a few other selectors that needs to be fixed, check out the diff on the generated css file, anything with > * > * looks suspicious and probably needs to be fixed.

Addressed #35, also fixed some logical properties. Attached interdiff for same.

For the screenshots but looking at the patch

    border: 0.125rem solid;
    border-width: 0.125rem 0.125rem 0 0;

Is this not overriding itself?

Believe this is good to go.

+++ b/core/themes/claro/css/components/dropbutton.css
@@ -324,7 +309,7 @@
-.dropbutton__item:first-of-type > *:active {
+.dropbutton__item:first-of-type:active {

Is this change intended?

No, this is not intended. I have fixed it. I have attached the patch and interdiff for same.

Committed 6b70d15 and pushed to 11.x. Thanks!

In 10.2.x version the code for .form-actions .dropbutton-wrapper was:

.form-actions .dropbutton-wrapper,
.field-actions .dropbutton-wrapper {
  margin: var(--space-xs) var(--space-m) var(--space-xs) 0;
}

With this code, the dropdown had proper alignment.

In 11.x version it has been changed to:

.form-actions .dropbutton-wrapper,
.field-actions .dropbutton-wrapper {
  margin-block: var(--space-xs);
  margin-inline: var(--space-m) 0;
}

And now it's breaking the dropdown alignment.

Steps to reproduce it:
- Install the contrib module Workbench Moderation →
- Now after enabling the module edit any content type inside the structure and enable the moderation from the Manage moderation tab.
- Now create new content of the respective content type and now you can see the issue.

Creating a patch file instead of raising an MR as I am getting issues while creating a new branch from 11.x branch.

Hi @AkshayAdhav,
The alignment issue is handling in this - https://www.drupal.org/project/drupal/issues/3391800 🐛 Dropdown button display becomes narrow when text is long Needs work
Thanks

Hi @shweta__sharma
The issue you have mentioned is about vertical displacement along with the issue of the button's text length.
As this particular issue was closed with wrong property values, it was adding left margin instead of right margin. You can check the screenshots attached in my comment #45 📌 Refactor Claro's dropbutton stylesheet Needs review .

Ohh! Thanks, @AkshayAdhav for the clarity. I tested your MR 5859 on Drupal 11 the MR is cleanly applied and the issue is fixed. The space is added between buttons now. Attached screenshot for reference.

I'm triaging RTBC issues → . I read the IS and the comments.

This was committed to 11.x and then re-opened for a problem with the contrib module, Workbench Moderation. My first question is this reproducible with just Drupal core? The same comment mentions 10.2.x, so is this a problem only on 10.2?

I have found that opening Fixed issues makes it difficult to track history of problems, so the discussion from #45 onwards may need to move to a new issue.

Committed 39fb7eb and pushed to 11.x. Thanks!

I opted to commit it from here since this is a simple mistake and it should have been part of the initial patch. I do not see it as a sign there is a deeper issue with the previous patch.

and to anwser the questions.

The problem is not present in 10.2 because it doesn't have the refactor, 10.2 version was used as an example of what the code and behavior should be.
It is probably not possible to replicate with core since we got rid of dropbuttons on those kind of submits a while back.

@nod_, thanks!

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