Update Link UI design for preview action buttons to use icons for edit and unlink #35833
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
getdave
added
the
[Feature] Link Editing
|
Size Change: +304 B (0%) Total Size: 1.07 MB
ℹ️ View Unchanged
|
|
Nice one, here's a GIF of the two buttons in action: Looks good, works well. Technically unrelated to this PR, so feel free to ignore, but the link card descriptio has It looks like that color is coming directly from the components themselves, and it seems like those components should not be setting colors to anything but inherit. @ciampo would this be an easy fix? Also not directly related, but the focus style of the link is both cropped and the oldschool wp-admin link style:
This is probably also best fixed or ticketed separately. It might even look different in the site editor already, since that's iframed. Nice work! What do you think? I'd be happy to give a green check on this one as is. |
|
Thanks for review @jasmussen.
Happy to address both the items you noticed as follow ups. Should be simple enough. I also need to fix/update the tests. |
|
Mostly to clarify, it doesn't necessarily have to be your responsibility to catch all of those. But for the purposes of the review, I think it's worth calling the various glitches out explicitly, as it might not be obvious that the glitch is a separate issue. |
7 tasks
Addressed in #35851
Addressed in #35853
Nope it looks the same for whatever reason (see screenshots above).
I didn't fully understand this. I think you're saying I should call out things like this before I request PR review. I agree, but unfortunately my poor design eyes didn't spot these details 😄 More than happy to address them though. |
7 tasks
Not at all, you're doing excellent work and I'm trying to clarify you don't have to fix everything under the sun, we're all here to help carry the load. What I'm also saying is that I have a habit of looking at the full flow when testing these PRs, and in the process I will often encounter small issues that are outside of the scope of a particular PR to fix. It isn't always obvious why a bug that is visible in a PR review is not fixed as part of that same PR, so explaining why, I find, can be helpful. For example in the focus style PR you just did, it became very clear that updating from the blurry gray-blue box shadow to the new crisp theme-color solid style, that is something to fix in wp-admin itself, not Gutenberg. |
|
Thanks @jasmussen
Do we have a 👍 on this one in that case? |
jasmussen
approved these changes
Oct 25, 2021
getdave
force-pushed
the
update/new-design-for-link-preview-action-buttons
branch
from
e40dc69
to
47b8134
Compare
|
Thank you! 🙇 Rebasing now (in hope of fixing seemingly unrelated randomly failing e2e test) with a view to merge. |
This reverts commit 3d84721.
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.
Description
In #33849 (comment) @jasmussen proposed a new design for the Link UI's preview mode.
This PR implements part of that design by normalising the placement of the edit and unlink buttons.
Note that the work to add the split text/url fields is under way in as separate PR #33849.
How has this been tested?
Screenshots
Before
After
Types of changes
New feature (non-breaking change which adds functionality)
Checklist:
*.native.jsfiles for terms that need renaming or removal).