Reduce minimum spacer height to 1px, while still retaining clickable area #25528
Conversation
This PR takes a different approach to allowing a 1px tall spacer block. The primary problem is that the sibling inserter above and below a 1px spacer block makes it appear hard to select the spacer. This PR addresses that primarily by changing the cursor from being hardcoded to `text`, to not being changed at all. In practice that means when you try to click a 1px spacer, you're technically clicking the sibling inserter hover area, but that area redirects to the nearest block which in most cases means the spacer block.
jasmussen
requested review from
ajitbohra,
ellatrix,
Soean,
talldan and
youknowriad
as code owners
|
Size Change: +70 B (0%) Total Size: 1.2 MB
ℹ️ View Unchanged
|
|
Thanks so much. I'm out for the day but I'll be back on this tomorrow and hopefully pairing with @talldan on a separate/additional range control fix that only validates/corrects the minimum value once focus is outside of the input field. |
Hmm... sounds awfully familiar #25131 😉 |
|
:mindblown: CC @talldan |
|
Awesome, I've rephrased the body text to state the issue it fixes and only suggest it mitigates the other one. I've restarted the tests on this one, it looks like they should pass. So it seems like the next step is just to get a review for this one and hopefully land it. I'll take a look at reviewing #25131 next. |
There was a problem hiding this comment.
This is working really well in my testing. The spacer is still very easy to select.
The only thing I notice that could be improved still is that it's a little difficult to get the block's border to show in navigation mode, which makes selecting if feel difficult (even though the click area is bigger than the border portrays):
I think we should give this a go though and try to improve that separately. Great work @jasmussen and @stokesman and anyone else who has taken time to iterate on this, I think this will make a lot of people happy!
talldan
changed the title
|
When to expect this in the Gutenberg plugin for testing? |
|
Version 9.1 I'd think. |
|
@youknowriad After merging this I'm seeing some block validation issues with existing spacer blocks :( — did I do something wrong here? Do I need to to revert? |
|
This PR doesn't have any impact on block vallidation, it's probably something else. |
|
Working well, but with one limitaion:
|
|
@Sven74Muc, when you refer to the up and down arrows, I've been unable to reproduce the problem so far. What browser are you experiencing this with? |
|
Yes, I mean the little arrow buttons. |
|
I switched on the troubleshoot mode... disabled all plugins and switched to the twenty twenty theme >>> working |
|
@Sven74Muc Thanks for the report. I can't reproduce, though, even in Firefox. Do you have any additional steps to reproduce? It also works for me in TwentyTwenty |
|
Thank you @Sven74Muc. I can confirm the issue. Easily reproduced in Firefox by clicking on the little arrows anytime the input does not already have focus. Other browsers focus the input at the same time the arrows are clicked. Definitely not an issue with this PR. I'll try making a fix since it relates to changes from #25609. |
|
Great, thanks @stokesman |
|
Just wondering where you have the arrows... I have them only on the right sidebar, not directly at the block |
|
Ah, got it.... I mean different arrows, please see here: https://dlgo.de/30-09-_2020_20-15-09.mp4 |
|
@Sven74Muc, no worries. When I mentioned that it's not an issue with this PR it was to clarify for other readers. I've drafted a PR that fixes the issue. I'd guess this will be resolved by the next release but I'm not one to say. |
|
Thanks @stokesman |
This PR takes a different approach to allowing a 1px tall spacer block. Alternative to #25525. Fixes #18906. Mitigates #24460.
The primary problem is that the sibling inserter above and below a 1px spacer block makes it appear hard to select the spacer.
It does a few things. First off, through a line height it fixes an issue with the sibling inserter being too tall. That red area is the clickable area shown for debug purposes:
Fixed:
It alos changes the sibling inserter cursor from being hardcoded to
text, to not being changed at all. In practice that means when you try to click a 1px spacer, you're technically clicking the sibling inserter hover area, but that area redirects to the nearest block which in most cases means the spacer block. So this is more of a visual change — when the cursor was text it didn't seem like a click would select the spacer block when in fact it would:And finally, it adds a hidden "sizer" above the spacer block which provides a minimum clickable area:
Here, it's shown in red, that's only for debug purposes. I believe we tried this in the past and it didn't work, but it seems to work fine here:
CC and props: @stokesman for the initial work.