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.
We are down to the final must haves for #2762505: Introduce "outside in" quick edit pattern to core
#2919373: Prevent Settings Tray functionality for blocks that have configuration overrides#2924351: Fix coding standards issues with existing settings tray JavaScript#2773601: Display "You are now in edit mode" prompt when user enters edit mode#2893937: Complete Handbook documentation for Settings Tray moduleManually test in all core themes.#2928409: [META] Accessibility issues for Settings Tray#2897272: Fix module description, hook_help(), and document module scope in *.api.php file
Patch items for this issue
- Copy CSS files to Stable theme
After that Settings Tray should consider stable. Let get it done!
Comment | File | Size | Author |
---|---|---|---|
#21 | interdiff-14-21.txt | 7.19 KB | tedbow |
#21 | 2922603-21.patch | 9.63 KB | tedbow |
#14 | 2922603-14.patch | 2.44 KB | tedbow |
Comments
Comment #2
tedbowComment #3
tedbowComment #4
GrandmaGlassesRopeManComment #5
GrandmaGlassesRopeManComment #6
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer and at Annertech commentedTagging for my list.
I'm aware of a few outstanding a11y points which have been mentioned in various places, including some issues focused on the layout builder. We need to gather those up, and make separate actionable issues where appropriate.
I've also seen a lot of issues crop up (e.g. in the accessibility tag), where it's clear accessibility is being addressed by the settings tray contributors themselves, which is GREAT :-)
@tedbow:
A11y review & sign-off could be quicker if the maintainer walks me though the architecture, and point out what's been done for a11y already. I was following it during the very early stages, but lost track. In particular, I'm keen to learn how the dialog and off-canvas components differ now that they are separate. Could we find a time to meet in Slack/Skype/Hangouts for this?
Comment #7
tedbow@andrewmacpherson thanks for tagging for your list.
Yes definitely! Just contact via Drupal Slack, IRC, or my d.o contact form and we can setup a time.
Comment #8
tedbowComment #9
tedbowForgot to add #2897272: Fix module description, hook_help(), and document module scope in *.api.php file to this list
Comment #10
tedbowReviewed #2773601: Display "You are now in edit mode" prompt when user enters edit mode with UX meeting. All agreed this should not a be blocker for the module being stable(and it might not be needed at all).
Comment #11
tedbowStriking through issues that are done in the summary
We had
But the only core themes that available with testing flag turned on are Bartik, Seven and Stark.
I just tested Bartik and Stark. Seven doesn't apply because it removes contextual links.
The functional javascript testing in the module does cover 'bartik', 'stark', 'classy', 'stable'
Comment #12
tedbow#2922603: Mark Settings Tray module as stable committed!!!
#2928409: [META] Accessibility issues for Settings Tray also just got it's last MUST-HAVE issue done: #2934178: Contrast issues with off-canvas dialog styling
So crossing if off to on this list as it should not be a blocker for Stable any more.
This leaves only #2919373: Prevent Settings Tray functionality for blocks that have configuration overrides which was RTBC but after a demo with @webchick it is not back to Needs Review for a patch addressing some changes she recommended.
Comment #14
tedbowYay! #2919373: Prevent Settings Tray functionality for blocks that have configuration overrides was committed! There was small follow up #2938309: Only install Quick Edit when necessary for Settings Tray tests because of test failures with PostgreSQL
We probably also want to do #2896356: Move 'settings_tray' forms out of Settings Tray and into respective modules and annotations before this current issue but #2896356 only should be done if the module is going stable.
So I don't know if it would make sense to make it part of this patch. But leaving out for now this patch is smaller.
Comment #15
Wim Leers🎉
Comment #16
tedbowI should have remove "needs accessibility review" tag in #12, removed
Comment #17
tedbowComment #18
Wim LeersCongrats!
Comment #19
xjmThanks @Wim Leers and @tedbow!
This seems to be in good shape. We'll look at this again next month. (Marking postponed for now; it can just be set back to RTBC on Feb. 21.)
Comment #20
xjmTalked to catch and larowlan; we agreed we can go ahead with this now. Setting back to RTBC. :)
Comment #21
tedbowI talked with @xjm who recommended that we bring in the patch in #2896356: Move 'settings_tray' forms out of Settings Tray and into respective modules and annotations (which is also RTBC) into this issue because those changes should only happen when the module goes stable.
Here is the combined patch.
Note to future self:
To make git find the copies use
git diff -M10% --find-copies-harder 8.6.x > ../2922603-21.patch
and set make sure gitconfig has
diff.renames=copies
Comment #22
xjmAdding credit to @Wim Leers here. His contribution on this issue is emoji but he reviewed the other. :)
Comment #23
xjmIt would be good to get a last +1 on this since I proposed the latest change and Ted rolled a patch. I pinged @tim.plunkett for a quick look since it touches core block implementations.
Comment #24
tim.plunkettI personally don't like having the settings_tray keys in system.module blocks, but there is precedence for "use some optional module's annotation key in a core annotation": the
field_ui_base_route
flag on entity types.So given that, I think this is fine. Re-RTBC.
Comment #26
xjmThanks @tim.plunkett for the confirmation and slightly reluctant signoff. :)
The committers have discussed Settings Tray's status in several previous meetings and again in Slack today, and each time those discussing it were comfortable with Settings Tray being stable once the above issues were resolved, including backporting this between 8.5.0's alpha and beta (i.e. right now).
So let's do this!! ☃️💯💃🏻
Fixed the following on commit:
Like so:
Congratulations @tedbow and contributors!