Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Follow-up from #2055853: [meta] Improve the place block UX; Separate interaction from the create block UX; Improve the existing blocks-by-theme layout.
When adding a block from the modal, the newly added block is often hard to find visually, especially if you forget to fill out a region for it.
This colors the row of a just-placed block, and also scrolls it into view.
A follow-up: #2088027: just placed block keeps changed color even after saving the block layout page
Comment | File | Size | Author |
---|---|---|---|
#17 | core-block-ui-highlight-2078951-17.patch | 1.54 KB | nod_ |
#10 | block-highlight-row-2078951-10.patch | 5.91 KB | jessebeach |
#10 | interdiff.txt | 975 bytes | jessebeach |
#9 | interdiff.txt | 2.15 KB | tim.plunkett |
#9 | block-2078951-9.patch | 5.72 KB | tim.plunkett |
Comments
Comment #1
tim.plunkettDidn't mean to copy all those tags.
Comment #2
tim.plunkettAFAIK this can't be tested with simpletest.
Comment #3
Bojhan CreditAttribution: Bojhan commentedWhoo :) This is soooo critical and an issue we identified + solution we actually identified back in 2009 at the Baltimore usability test. Very happy to see this patch :)
I do think that choosing a region should become required. Although the UX is a little wonky, the whole disabled region concept is even wonkier.
Comment #4
tim.plunkettAhhhh crap. This won't work with the overlay at all.
Anyone have a good suggestion about how to get the information from one page to the next? Local storage?
Comment #5
tim.plunkettHere's a different approach.
There is already a bug with closing a modal while in the overlay, I'll open a separate issue for that.
Comment #6
tim.plunkettThe nice thing about this approach is that the color highlighting will still work without JS on, you just won't get the scroll animation.
Comment #7
tim.plunkettRerolled, removed the Drupal:: call
Comment #9
tim.plunkettFor me, #2076411: Remove the request scope from the current user service breaks the toolbar's JS. If this patch doesn't work during manual testing, you probably have
Uncaught SyntaxError: Unexpected token <
in your JS console. Disable the toolbar, comment on that issue about how broken it is, and continue testing :)I forgot there is a route controller sitting in front of the list controller, I have to pass the request all the way through.
Comment #10
jessebeach CreditAttribution: jessebeach commentedI manually tested the patch on multiple browsers. It works.
I have one small change to the JS that introduces
once()
. Otherwise that screen will scroll to the just-placed block any time an AJAX request is sent off or something else invokesDrupal.attachBehaviors
.Tim, just have a look at the JS I changed. If you're fine with it, this is ready to be RTBC pending the bot.
Comment #11
tim.plunkettThanks, I always forget about .once() until someone reports a weird bug, then I remember it!
+1, leaving for someone to RTBC.
Comment #12
Wim LeersI tried to break this during manual testing, but could not :)
I tried to find nitpicks, but could not :)
Much nicer UX indeed.
Comment #13
Dries CreditAttribution: Dries commentedCommitted to 8.x. Thanks.
Comment #14
nod_tag.
Also, animate() #sadpanda. and using a function as an argument to .once() and doing a var $thing = $(this) when you're already creating the jquery object the line before #sadunicorn.
Comment #15
tim.plunkettSo like this?
I'm not sure what the replacement for .animate() would be, and I honestly don't know what you mean when you say you're sad about "using a function as an argument to .once() ".
Comment #16
nod_Almost:
When we'll replace jquery with regular JS it'll be easier to change things when this form is used instead of the function thing. Which also creates a new scope that really isn't needed usually.
For the animate thing I don't know haven't saw the D8 UI in weeks so maybe it's needed, but I took out the animation for collapsible fieldsets a while back, I hoping the same can happen here (or at least it was supposed to be removed in #2060573: Standardized filter/search behavior; surface filter matches regardless of their containing box's open state; but anyway).
Comment #17
nod_Reopening.
That kind of stuff is handled by 20+ years old technology (we broke)
#block-placed
. This patch + #1870006: HTML5 validation with table sticky header is misaligned over the toolbar should be enough.Comment #19
tim.plunkettI'm manually testing this in conjunction with #1870006: HTML5 validation with table sticky header is misaligned over the toolbar, and I see different behavior with and without a modal.
Starting on
http://d8/#overlay=admin/structure/block
, using the modal link to place a block, I end up onhttp://d8/admin/structure/block/list/bartik?block-placement=bartikadministration-2#block-placed
, because submitting a modal form apparently kills the overlay.Starting on
http://d8/#overlay=admin/structure/block/add/system_menu_block%3Afooter/bartik
(bypassing the modal, but still in the overlay, I end up onhttp://d8/#overlay=admin/structure/block/list/bartik%3Fblock-placement%3Dbartikfooter-2%23block-placed
, which has the second # encoded, and obviously does not work for either the scroll or for the addition of the class and ID anchor itself.Comment #20
nod_I'll open follow-up then.
Comment #21
YesCT CreditAttribution: YesCT commentedfollow-up: #2088027: just placed block keeps changed color even after saving the block layout page
Comment #22.0
(not verified) CreditAttribution: commentedUpdated issue summary.