List View: Update drop indicator line to be 4px high with a white border #49462
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andrewserong
added
[Type] Enhancement
andrewserong
requested review from
talldan,
jasmussen,
apeatling and
tellthemachines
|
Size Change: +531 B (0%) Total Size: 1.34 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 439cdab. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4570128110
|
|
Looks good! Thanks for following up:
That line is only 2px though (4px@2x of course). This may actually be preferrable given the list view. But I'm wondering if it looks too clunky if we go full 4px (8px@2x) like this:
Conceptually the indicator is the same UI, so if they matched visually and it wasn't clumsy, that might be nice. |
|
I think this may also close #39068. |
apeatling
approved these changes
Mar 30, 2023
There was a problem hiding this comment.
Works great! LGTM after the adjustments from @jasmussen are in a good spot.
|
Thanks for the reviews and feedback! I've had a go at increasing the height to 2023-03-31.11.07.45.mp4To my eye, this is now visually consistent with the line in the editor canvas. It's fairly thick now, but it's definitely much clearer for a user. If this looks good to you @jasmussen, feel free to merge this PR in. Otherwise, I'm happy to keep tweaking next week 🙂 |
|
Let's try this. From my POV there are only benefits to visually unifying this as a single style, rather than two different styles. |
|
Going to merge this for you @andrewserong 😎 |
apeatling
deleted the
try/tweak-styling-of-list-view-drop-indicator-line
branch
|
Thanks for the reviews, and for merging, Andy! 🙇 |
25 tasks
bph
added
[Type] Bug
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Possible fix for: #39068
Explore updating the drop indicator line styling in the list view to be 4px high with a white border. Currently the line is only
1pxin size and is fairly subtle. Also, it can be hard to see against a selected block, as selected blocks use the same background color as the drop indicator line.Based on a suggestion from @jasmussen in this review: #49390 (comment)
Why?
The current 1px line is very subtle, and it's also difficult to see when it sits against selected blocks.
How?
6px(with the white border, this has the appearance of being4pxhigh)4pxradius (with the white border, this has the appearance of being2px)Happy for any feedback or ideas on how to improve the styling if this isn't quite right!
Testing Instructions
Screenshots or screencast
Note: not addressed in this PR — when you start dragging a block, the color of the selected blocks switches to no longer be white. I can look at that issue in a separate PR.
2023-03-31.11.07.45.mp4