Problem/Motivation

The 'perspective tabs' (content/behavior) are hardcoded and cannot be adjusted to different themes.

The tabs don't seem to render well on Claro or Gin (Claro subtheme).

Steps to reproduce

Create a paragraph with a behavior plugin and check with Claro / Gin themes.

Before Claro

Proposed resolution

1. Rendering the tabs through the theme layer.
2. Altering the 'is-active' class so it applies to the 'tabs__link' inside the <li> element, like the rest of Drupal.

Remaining tasks

Fix broken tests (if any).

CommentFileSizeAuthor
#26 3199551_25-26-interdiff.txt1.42 KBmathilde_dumond
#26 3199551_26.patch3.41 KBmathilde_dumond
#25 3199551_23-25-interdiff.txt1.02 KBmathilde_dumond
#25 3199551_25_claro_behaviorTabs.patch3.39 KBmathilde_dumond
#23 3199551_23_claro_behaviorTabs.patch2.59 KBmrinalini9
#21 3199551_interdiff_18_21.txt640 bytesmathilde_dumond
#21 3199551_21_claro_behaviorTabs.patch4.18 KBmathilde_dumond
#18 paragraphs_3199551_18_claro_behaviorTabs.patch3.22 KBromina_ferrario
#16 Screenshot_ParagraphTabs_Mobile.png20.47 KBromina_ferrario
#16 Screenshot_ParagraphTabs_Desktop.png22.72 KBromina_ferrario
#16 paragraphs_3199551_16_claro_behaviorTabs.patch3.56 KBromina_ferrario
#15 Screenshot_claroStyleFocus.png10.39 KBromina_ferrario
#15 Screenshot_claroStyleHover.png7.53 KBromina_ferrario
#15 Screenshot_claroStyle.png14.09 KBromina_ferrario
#15 Screenshot_brokenMobileStyling-Claro.png11.33 KBromina_ferrario
#15 Screenshot_sevenWithThisPatchHover.png6.03 KBromina_ferrario
#15 Screenshot_sevenWithThisPatch.png8.58 KBromina_ferrario
#15 Screenshot_currentSevenHover.png5.88 KBromina_ferrario
#15 Screenshot_currentSeven.png7.87 KBromina_ferrario
#15 paragraphs_3199551_15_claro_behavior.patch2.16 KBromina_ferrario
#13 3199551-13.patch5.95 KBJeroenT
#12 Screenshot 2021-10-15 at 10.05.19.png90.28 KBJeroenT
#12 Screenshot 2021-10-15 at 10.05.13.png65.92 KBJeroenT
#12 3199551-12.patch5.91 KBJeroenT
#11 Screenshot 2021-09-16 at 14.04.18.png57.36 KBJeroenT
#7 3199551-7.patch9.47 KBnuez
#6 after_claro.jpg88.77 KBnuez
#2 claro_before.jpg149.1 KBnuez

Issue fork paragraphs-3199551

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

nuez created an issue. See original summary.

nuez’s picture

nuez
Primary language Dutch
Location Madrid, Spain
CreditAttribution: nuez for Expload BV commented
Issue summary: View changes
FileSize
claro_before.jpg149.1 KB
nuez’s picture

nuez
Primary language Dutch
Location Madrid, Spain
CreditAttribution: nuez for Expload BV commented
Issue summary: View changes

nuez opened merge request !7

nuez’s picture

nuez
Primary language Dutch
Location Madrid, Spain
CreditAttribution: nuez for Expload BV commented
Status: Active » Needs review
nuez’s picture

nuez
Primary language Dutch
Location Madrid, Spain
CreditAttribution: nuez for Expload BV commented
FileSize
after_claro.jpg88.77 KB

1. Added the 'is-active' class to the link instead of the <li> element- with all corresponding JS changes.
2. Wrapped the tabs in a div, so the tabs can be visualised using display:flex (which is what Claro/Gin uses)
3. Put the tabs in a theme function. Although the tabs work with claro now, other teams might want to alter styling even further.
4. Added $.blur() because the tabs were 'hanging in' a ':focus' state causing wrong styling.

This is after the fix:

After Claro

nuez’s picture

nuez
Primary language Dutch
Location Madrid, Spain
CreditAttribution: nuez for Expload BV commented
FileSize
3199551-7.patch9.47 KB

Not sure if there's a better way, but also uploading the patch to include it in composer.

nuez’s picture

nuez
Primary language Dutch
Location Madrid, Spain
CreditAttribution: nuez for Expload BV commented
Title: The conent/behavior tabs on the paragraphs widget don't render properly in Claro. » The content/behavior tabs on the paragraphs widget don't render properly in Claro.

Status: Needs review » Needs work

The last submitted patch, 7: 3199551-7.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

JeroenT made their first commit to this issue’s fork.

JeroenT’s picture

JeroenT
Primary language Dutch
Location 🇧🇪
CreditAttribution: JeroenT at iO commented
FileSize
Screenshot 2021-09-16 at 14.04.18.png57.36 KB

The desktop version looks great!

Tests seems to be failing because the tabs are no longer sticky at the top of the page.
Also the tabs are no longer shown on mobile screens:

JeroenT’s picture

JeroenT
Primary language Dutch
Location 🇧🇪
CreditAttribution: JeroenT at iO commented
Status: Needs work » Needs review
FileSize
3199551-12.patch5.91 KB
Screenshot 2021-10-15 at 10.05.13.png65.92 KB
Screenshot 2021-10-15 at 10.05.19.png90.28 KB

I took a bit of a different approach.

I removed the tab classes and created dedicated styling for this element.
The advantage of this approach is that every theme uses these styles as default.
Every theme that wants to improve these element, can override the styles.

Seven:

Gin (or Claro):

JeroenT’s picture

JeroenT
Primary language Dutch
Location 🇧🇪
CreditAttribution: JeroenT at iO commented
FileSize
3199551-13.patch5.95 KB
JeroenT’s picture

JeroenT
Primary language Dutch
Location 🇧🇪
CreditAttribution: JeroenT at iO commented
Assigned: nuez » Unassigned
romina_ferrario’s picture

romina_ferrario CreditAttribution: romina_ferrario at MD Systems GmbH commented
Status: Needs review » Needs work
FileSize
paragraphs_3199551_15_claro_behavior.patch2.16 KB
Screenshot_currentSeven.png7.87 KB
Screenshot_currentSevenHover.png5.88 KB
Screenshot_sevenWithThisPatch.png8.58 KB
Screenshot_sevenWithThisPatchHover.png6.03 KB
Screenshot_brokenMobileStyling-Claro.png11.33 KB
Screenshot_claroStyle.png14.09 KB
Screenshot_claroStyleHover.png7.53 KB
Screenshot_claroStyleFocus.png10.39 KB

Started from the beginning and created new patch.
This is how claro looks like with this patch (based on the claro styleguide):
active and not active state:
screenshot of active and none active state on claro

hover:
screenshot of hover state on claro

focus:
screenshot of focus state on claro


NOTE:

- Seven looks different with this patch but it's not broken.

Seven now:
active and not active state:
screenshot of current state of seven

hover:
screenshot of current hover state of seven

Seven after add the patch:
active and not active state:
screenshot of state with path on seven

hover:
screenshot of hover state with path on seven

- The mobile view is broken now. I will continue to work on it and fix the mobile view.
screenshot of broken mobile view

romina_ferrario’s picture

romina_ferrario CreditAttribution: romina_ferrario at MD Systems GmbH commented
FileSize
paragraphs_3199551_16_claro_behaviorTabs.patch3.56 KB
Screenshot_ParagraphTabs_Desktop.png22.72 KB
Screenshot_ParagraphTabs_Mobile.png20.47 KB

added a new patch in which desktop and mobile design is fixed

Desktop view:
Screenshot Paragraph Tabs Desktop

Mobile view:
Screenshot Paragraph Tabs Mobile.png

JeroenT’s picture

JeroenT
Primary language Dutch
Location 🇧🇪
CreditAttribution: JeroenT at iO commented
Status: Needs work » Needs review
romina_ferrario’s picture

romina_ferrario CreditAttribution: romina_ferrario at MD Systems GmbH commented
FileSize
paragraphs_3199551_18_claro_behaviorTabs.patch3.22 KB

created a new patch because I found the classes on claro which includes all styling (for mobile and desktop). Also with this patch seven looks exactly the same as bevor.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented 
+++ b/css/paragraphs.widget.css
@@ -137,10 +137,6 @@
-.paragraphs-tabs-wrapper .paragraphs-tabs {
-  display: none;
-}

why is this removed? isn't this for cases when there are no behaviors? make sure it only shows up when you add paragraph types that have behaviors.

romina_ferrario’s picture

romina_ferrario CreditAttribution: romina_ferrario at MD Systems GmbH commented

I'm not sure if this is originally written for that, but it's no longer needed. Also not to hide the tabs when there is no Behavior.

mathilde_dumond’s picture

mathilde_dumond CreditAttribution: mathilde_dumond at MD Systems GmbH commented
FileSize
3199551_21_claro_behaviorTabs.patch4.18 KB
3199551_interdiff_18_21.txt640 bytes

with the display: none rule added back-

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Needs review » Needs work

Patch doesn't apply on 8.x-1.x-dev.

mrinalini9’s picture

mrinalini9 CreditAttribution: mrinalini9 at Srijan | A Material+ Company for Drupal India Association commented
Status: Needs work » Needs review
FileSize
3199551_23_claro_behaviorTabs.patch2.59 KB

Rerolled patch #21, please review it.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Needs review » Needs work

Discussed, the none css breaks the display: flex through show(). lets change paragraphs-tabs to paragraphs-hide for the none selector and remove/add that class in our JS.

mathilde_dumond’s picture

mathilde_dumond CreditAttribution: mathilde_dumond at MD Systems GmbH commented
Status: Needs work » Needs review
FileSize
3199551_25_claro_behaviorTabs.patch3.39 KB
3199551_23-25-interdiff.txt1.02 KB
mathilde_dumond’s picture

mathilde_dumond CreditAttribution: mathilde_dumond at MD Systems GmbH commented
FileSize
3199551_26.patch3.41 KB
3199551_25-26-interdiff.txt1.42 KB

now with the class hide applied from the start

  • Berdir committed 45a4dbf on 8.x-1.x 
    Issue #3199551 by romina_ferrario, JeroenT, mathilde_dumond, nuez,...
Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Needs review » Fixed

Committed.

Status: Fixed » Closed (fixed)

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