Format Library: Add alt edit field to inline image #64124
Merged
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
t-hamano
added
[Type] Enhancement
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: +483 B (+0.03%) Total Size: 1.76 MB
ℹ️ View Unchanged
|
jasmussen
approved these changes
Jul 31, 2024
There was a problem hiding this comment.
This could use some style improvements. But whether they happen here or as a followup, that can be decided separately.
Mainly, the input sizes, spacings, could use some tweaks. The modal shows a 12px gap between last input, and the confirm button:
I think that should actually be 16, but that's separate. But that same gap should also be the gap between the width control, and the alt text control, right now they're very close together:
Question: should the same help text be present for this alt-text field?
Nice work.
ellatrix
reviewed
Jul 31, 2024
| value={ alt } | ||
| onChange={ ( newAlt ) => { | ||
| setAlt( newAlt ); | ||
| setHasChanged( true ); |
There was a problem hiding this comment.
Why is this needed? Can't we compare the current state?
I was worried that the popover would be too big, but I gave it a try. What do you think?
Update:
Changed the gap to 16px.
|
|
Flaky tests detected in 7e17619. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/10177667547
|
|
If it becomes too big, maybe there should be a tab in the inspector for inline content. I think @dmsnell was also looking for that for Bits. :) |
ellatrix
reviewed
Jul 31, 2024
| { __( 'Apply' ) } | ||
| </Button> | ||
| </HStack> | ||
| </VStack> |
There was a problem hiding this comment.
Looking at this deeper, why do we even have an apply button here? The image block does not have this either, it just updates right away.
There was a problem hiding this comment.
Even in trunk, it won't update unless you explicitly submit the form or press the apply icon button. If we update it immediately, I think the popover will rattle 😅
2b809508a0a1330326ffa2cbe00a3f7c.mp4
ellatrix
approved these changes
Jul 31, 2024
|
@jasmussen Please let us know if there is anything we can improve further in terms of design in this PR 🙏 Below are the latest screenshots:
|
Here's what I'm seeing:
I pushed two small changes to ensure this:
|
|
@richtabor Thanks for the update! |
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.
Fixes #63323
Related to #28813
What?
This PR makes it possible to change the alt text in the inline image popover.
Why?
If the media already has alt text set, the alt attribute will be applied to the inserted inline image. However, if you want to change the alt attribute, you have no choice but to change the alt of the original media and then reinsert it, or change the value directly via the code editor.
How?
Add a text area to the popover to change the alt text.
Testing Instructions
Testing Instructions for Keyboard
You should be able to change the alt text using just the keyboard.
Screenshots or screencast
f60c149851518f28ed68ff4aa7087883.mp4