Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
To stick to the design system approach for claro the implementation for components should stick to predefined variants.
Proposed resolution
Since Add support for Dropbutton variants (3057581) is moved forward claro can rely on it.
Remaining tasks
- Test patch
- Figure out how to solve views display settings: https://git.drupalcode.org/project/claro/blob/8.x-1.x/claro.theme#L435 (addressing in #1899236: Add new Splitbutton render element to eventually replace Dropbutton)
Comment | File | Size | Author |
---|---|---|---|
#39 | 3060697-39.patch | 10.78 KB | Lal_ |
#37 | interdiff_35-37.txt | 1.33 KB | bnjmnm |
#37 | 3060697-37.patch | 10.75 KB | bnjmnm |
#35 | 3060697-35.patch | 10.75 KB | bnjmnm |
#35 | interdiff_30-35.txt | 1.64 KB | bnjmnm |
Comments
Comment #2
quironThis is an initial patch for early review.
Comment #3
lauriiiPostponed this until #3057581: Add support for Dropbutton variants gets done.
Comment #4
ccasals CreditAttribution: ccasals at Phase2 commentedManual testing of the patch resulted in a failure to apply on latest dev build.
Comment for the googlers out there - dropbuttons for things like the paragraphs widget break in both the alpha and latest dev release, I believe as a result of this issue. Using a different form widget is a work around.
Comment #5
Wim Leers#3057581: Add support for Dropbutton variants is RTBc, hopefully this is soon unblocked 🤞
Comment #6
lauriii#3057581: Add support for Dropbutton variants is in. Marking this for review.
Comment #7
Wim LeersComment #8
huzookaComment #9
shashikant_chauhan CreditAttribution: shashikant_chauhan as a volunteer commentedRerolled the patch, based on patch in #2
Comment #10
shashikant_chauhan CreditAttribution: shashikant_chauhan as a volunteer commentedComment #12
huzookaComment #13
huzookaI tried to create an interdiff, but it won't make any sense...
Comment #15
huzookaComment #16
huzookaComment #17
lauriiiWould it be possible to explain why this is needed?
Otherwise this looks good 👏I cleared caches and tested both tables with operations and Views UI and they work as expected. ✨
Comment #18
huzookaComment #19
huzookaComment #20
huzookaAddressing #17.
Comment #21
lauriiiThank you for adding the clarification on why it was needed. It makes sense now ✌️
Tested manually to confirm that there are no visual changes on the content list or block UI and everything seems to work as expected 👏
Comment #22
huzookaComment #23
huzookaRebased #20.Comment #24
huzookaRebased patch #20.
Comment #25
huzookaComment #26
huzookaComment #27
lauriiiThe changes since #21 look good.
Comment #28
lauriiiComment #29
MeenakshiG CreditAttribution: MeenakshiG at OpenSense Labs for DrupalFit commentedComment #30
MeenakshiG CreditAttribution: MeenakshiG at OpenSense Labs for DrupalFit commentedComment #31
Devipriya Rajamanickam CreditAttribution: Devipriya Rajamanickam commentedComment #32
Devipriya Rajamanickam CreditAttribution: Devipriya Rajamanickam commentedComment #33
Percy101 CreditAttribution: Percy101 as a volunteer commentedThe patch applied cleanly
Comment #34
bnjmnmMoved this to 9.1 and the patch still applies cleanly.
trustedCallbacks
inClaroPreRender
not being fully acknowledged. A cache clear addressed this.I think this could be a little easier to understand if this is rearranged a little. Maybe start with "#dropbutton_type must be placed before the default renderer because..."
Comment #35
bnjmnmAddressing #34
Comment #36
tim.plunkettLooks good! Just two doc nits, the implementation is great.
add # to dropbutton_type
There's some odd wrapping on this comment, and missing () on the method names
Comment #37
bnjmnmTY @tim.plunkett This addresses #36
Comment #38
tim.plunkettThanks!
Comment #39
Lal_Re-rolling as it failed to apply
Comment #40
tim.plunkettThanks so much! Those post_updates clash so often.
Also, on a clean reroll, no need to unRTBC! Only if you made substantive changes (which isn't really a reroll) or were unsure about conflicts that needed resolving.
Comment #42
lauriiiCommitted 0551c86 and pushed to 9.1.x. Thanks!