MediaReplaceFlow: fix styling for LinkControl #47949
Conversation
brookewp
added
[Type] Bug
|
Size Change: +1 B (0%) Total Size: 1.33 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 39a8130. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4269153243
|
|
Thanks for tackling this, that's been a pretty poor experience for a long while. I think it's okay to make the menu a bit wider than the 220px it's been for a long time, but 328 feels too wide. Something in the 240/260 are would feel more right to me, just so the video and upload icons don't depart too far from the text on the left. Eliding very long URLs feels like the major win here. |
|
@brookewp Thanks for catching and handling this and for the ping. Can I ask is there anything we can do to lower specificity on LinkControl to avoid having the overrides? If not then this seems like a good solution 👍 |
Thanks for chiming in so quickly, @jasmussen! This is the result at 260px:
I'm happy to make this change, but I just noticed that the popover for 'Text tracks' (next to 'Replace' in the toolbar) has a width of 360px, with the spacing quite wide between the text and icon. Do you still prefer to keep the Replace popover smaller, or should we match this other popover in the same toolbar?
|
This is a good question, @getdave! 🤔 If we could separate the search action buttons from the drawer and we kept the width consistent with all gutenberg/packages/block-editor/src/components/link-control/style.scss Lines 459 to 467 in 9ca8b5e And I think even if we added a conditional to hide the drawer / cc @mirka or @ciampo, in case you have any input / ideas for removing overrides. |
I'm not super knowledgeable about I strongly agree that style overrides should be reduced — but Since these style overrides are already in place anyway, I would keep the scope of this PR small and focused on addressing the layout issue. |
|
Also, relatively to the layout of I created a codepen example to exemplify what I just wrote in the paragraph above: https://codepen.io/ciampo/pen/poOvxrd I still think that the changes in this PR are a good bandaid, but I also think that the real solution here is to rethink how |
That would be excellent. Is that ready for prime time in 2023? Guess it's possible to progressively enhance it. A lot of the whitespace handling is to fix bugs folks have raised so we'd need to be careful with that as well. |
|
Does it make sense to merge this PR as a quick fix in the meantime so it doesn't appear broken for users? With my upcoming schedule and current tasks, I don't have the bandwidth to go much further on this. So if we decide to go with one of @ciampo's suggestions, I'll leave it up to someone else to tackle. 🙂 |
|
I'd be in favour of merging @brookewp 's fix (after discussing any last tweaks that may be necessary), and then working on improving |
getdave
force-pushed
the
fix/mediareplaceflow-style
branch
from
32a9a9c
to
9ed6ab3
Compare
360 for text tracks feels too wide as well. 260 looks good for the URL. It would be fine to find a smaller width that works for both of those cases... maybe 300? Key is to not have it too wide, and to not have the text wrap in a way so that there's only a single word on the last line of text. |
brookewp
force-pushed
the
fix/mediareplaceflow-style
branch
from
9ed6ab3
to
39a8130
Compare
Great catch! This has been adjusted:
I think 300 looks much better! Here is the result:
As for Text Tracks... it's a bit messier, and these inconsistencies should probably be addressed elsewhere (and I'm sure many more cases could be addressed alongside this). The issue, in this case, is when editing the tracks; it gets a bit too squishy:
|
|
Cool! Lots more little fixes we can do to these dialogs, including the uniform input height. "Remove track" should probably also not be an underline button — I can't think of many cases where we should use that type of button at all, in fact. But that's separate. Thanks for the PR. |
brookewp
force-pushed
the
fix/mediareplaceflow-style
branch
from
39a8130
to
b9aa030
Compare
What?
While working on #38730, I came across an issue in Media blocks that contained
LinkControlthroughMediaReplaceFlow.The
text-overflow: ellipsiswasn't applied toblock-editor-link-control__search-item-infoinLinkPreviewbecause there wasn't a max-width set on the span.And due to the length, the link edit icon was cut off, and the Cancel/Apply button styles were broken as well:
Why?
With the edit icon cut off, it's likely difficult for users to find the edit option. The design is also inconsistent; the size changes from previewing to editing, and the buttons aren't in the correct place.
How?
Since there was already a fixed-width override on
block-editor-link-controlwithin theMediaReplaceFlow(#33995), the simplest and quickest option is to adjust the size to be wider, to show the icon, and to be consistent with the LinkControl popover size (360px) used in other cases:Unfortunately, with
MediaReplaceFlowusingLinkControl, it appears that the styles get a bit tangled. So we would also need to add overrides toblock-editor-link-control__toolsto prevent some of the styles from bleeding over from otherLinkControlcases, i.e. inline links in the Paragraph block.cc @getdave, with your recent work on
LinkControl#47310, I thought you might be interested to weigh in. 🙂Design feedback request
There is a bit of extra space for the Image block due to the way that
LinkPreviewhandles the length of the filename for images. I am not confident it would be a good idea to adjust the length because this affects many other areas usingLinkControl. I had originally considered proposing to use the filename instead of the URL for Media blocks so it's shorter like the Image block, but it wouldn't be consistent with other uses ofLinkControl(and I think this looks better).Thoughts? cc @jasmussen as I see you've worked on this before. 😄
Image Link Preview:
Testing Instructions
MediaReplaceFlowwith link options (Audio, Video)Screenshots
Video/Audio Link Preview
Video/Audio Link Edit