Problem/Motivation

We are experiencing bigger inconsistencies with paragraph buttons while using default Seven theme. In https://www.drupal.org/project/paragraphs/issues/3111075 we introduced the .button--small class to various buttons in order to display them correctly in Claro, but didn't notice these side effects.

For example, the dropdown buttons are very small, "Add paragraphs" button is small, while the Edit button is big.

Proposed resolution

Discuss how to tackle this issue and fix it properly. Maybe reset those paddings for Seven specifically?

Had a quick discussion with @saschaeggi on Slack. His words were "From a Design System point of view the Edit button in the table view should be a small one button--small but the Add paragraph ones can be regular ones", but I am guessing that this applies for Claro only?

Also while at it, the "Add more" button is missing the button--small class, we need to add it there too. See the screenshots attached.

Comments

sasanikolic created an issue. See original summary.

sasanikolic’s picture

Status: Active » Needs review
StatusFileSize
new1.71 KB
new124.71 KB
new126.78 KB
new138.43 KB
new156.19 KB

Here is an initial patch that resets the .button--small definition for the Seven theme and that adds that class to the field-add-more-submit button.

Also attaching a test with smaller edit buttons.

Status: Needs review » Needs work

The last submitted patch, 2: 3152922-small-paragraph-buttons-in-seven-theme.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

sasanikolic’s picture

Status: Needs work » Needs review
StatusFileSize
new2.59 KB
new898 bytes

This should fix the failing test.

pivica’s picture

Status: Needs review » Needs work

A bit of an unfortunate situation because module is applying CSS classes and fixes for themes instead of inverted situation where themes should be doing this. But what to do I guess it would be very hard for Claro theme to inject needed button--small CSS class on theme level.

I see more additional stuff from Claro like button--extrasmall CSS class added to paragraph code. This, and maybe more CSS classes we will add from Claro in future, could potentially create problems - it could randomly clash with contrib modules CSS rules, etc.

Anyway

  1. +++ b/css/paragraphs.actions.scss
    @@ -125,4 +125,15 @@
    +  @at-root #block-seven-content {
    ...
    +    .paragraphs-add-wrapper {
    

    Minor thing but, not sure should we use @at-root here to keep this in context of current scope - this is a fix that is fixing both current and .paragraphs-add-wrapper scopes. I would move this outside of current scope so we do not need to write @at-root.

  2. +++ b/css/paragraphs.actions.scss
    @@ -125,4 +125,15 @@
    +      .button {
    

    This rule is restoring regular button size because of applying `button--small` CSS class rule. Shouldn't we use here `button--small` instead of `button` and state in CSS selector exactly why this rule is applied?

Beside this, tested current patch and it does solve issues described in description.

sasanikolic’s picture

Thanks for the review @pivica. While I know the issues you mentioned above, I don't see any other possible solutions currently than to reset those styles in the paragraphs module.

Did those changes that you mentioned above now and found another case for the dropbuttons, fixed also those.

The last submitted patch, 6: 3152922-small-paragraph-buttons-in-seven-theme-6-NO-TEST.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

pivica’s picture

Changes looks good. Just one thingy:

+++ b/css/paragraphs.actions.css
@@ -127,8 +127,9 @@
+#block-seven-content .paragraphs-actions .button--small,
+#block-seven-content .paragraphs-add-wrapper .button--small,
+#block-seven-content .paragraphs-dropbutton-wrapper .button--small {

Instead of this can we just write:

#block-seven-content .button--small {

Tried this for first two selectors and it seems it works fine. I can't reproduce `.paragraphs-dropbutton-wrapper` selector but I guess it will also work?

sasanikolic’s picture

Right, I also think we can remove those selectors.

Status: Needs review » Needs work

The last submitted patch, 9: 3152922-small-paragraph-buttons-in-seven-theme-9-NO-TEST.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

berdir’s picture

+++ b/css/paragraphs.actions.scss
@@ -126,3 +126,12 @@
+// We added button-small class for the Claro theme UX definitions.
+// We therefore need to reset small button styles for the Seven theme.
+#block-seven-content {
+  .button--small {
+    padding: 4px 1.5em;
+    font-size: 0.875rem;
+  }

content is the name of the block id, while it is usually like this, it doesn't seem like a good idea to hardcode this. It's not nice, but what we could do is check the active them and then add a separate library with a separate file for seven specific overrides. in a follow-up, we could consider moving other seven specific styling in there as well.

if (\Drupal::theme()->getActiveTheme()->getName() == 'seven') {
add second seven specific library
}

sasanikolic’s picture

Moved the code to a separate paragraphs.seven.css file. Noticed that the selector is not strong enough, because seven's buttons.css is loaded after our css, so I had to make it more specific.

The last submitted patch, 12: 3152922-small-paragraph-buttons-in-seven-theme-12-NO-TEST.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

sasanikolic’s picture

Missed the buttons in the modal. Need to also change the selector there.

The last submitted patch, 14: 3152922-small-paragraph-buttons-in-seven-theme-14-NO-TEST.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

pivica’s picture

Status: Needs review » Reviewed & tested by the community

Checked latest changes in the patch, looks good.

Tested against Drupal 8.9.3 and Drupal 9.0.3 with latest paragraphs dev version and this patch. Buttons in Seven theme looks good, and I don't see any special problem with Claro related to buttons.

  • Berdir committed b668219 on 8.x-1.x authored by sasanikolic
    Issue #3152922 by sasanikolic: Small paragraph buttons in seven theme
    
berdir’s picture

Status: Reviewed & tested by the community » Fixed

Thanks everyone.

Status: Fixed » Closed (fixed)

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