Media & Text: Replace the deprecated __experimentalImageSizeControl with ResolutionTool #57540
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
carolinan
added
[Block] Media & Text
|
Size Change: +155 B (+0.01%) Total Size: 1.75 MB
ℹ️ View Unchanged
|
Move the media width control and update the "crop image to fill" label to better match the changes in #58447.
|
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. |
Remove the default value (true) from isShownByDefault in the ToolsPanelItem.
Import and apply the constants for the dropdownMenuProps that positions the popover for the options and reset panel.
t-hamano
reviewed
Mar 11, 2024
Indent the ToolsPanelItem for the Focal Point one more tab stop.
The condition that was used to show or hide the Media width control was removed in a previous pull request, but this was missed when trying to solve merge conflicts.
Set the default value for mediaWidth in resetAll() to 50. Make sure that only values other than the default (50) counts as a change to the media with setting.
The option is enabled by default, so it should also reset to the enabled (true) state.
|
The reason why I have sort of paused the work on this PR is because the Alt is currently not working for the featured image, Both should be solved, I just don't want to cause more merge conflicts. |
t-hamano
added
[Type] Enhancement
|
As mentioned in this comment, there are some concerns with making alt text editable when "Use featured image" is enabled. I don't mind going either way, but since this PR doesn't add new features or fix bugs, I think it's okay to start with this one first. |
|
I'm seeing this here. If we can keep the existing spacing/design then this works well.
|
|
Sorry for the late feedback.
I noticed that this |
|
Yeah I think something just went wrong trying to solve one of the merge conflicts. |
|
I have updated the reset and the missing tools panel item but it is not ready for review yet. |
t-hamano
reviewed
May 7, 2024
|
Well, I learnt that |
t-hamano
approved these changes
May 8, 2024
There was a problem hiding this comment.
LGTM!
- ✅ Items whose values have changed from their default values can be reset to their default values individually from the Settings option popover.
- ✅ Can return all values to their default values using the "Reset all" button in the Settings option popover.
- ✅ If "Use featured image" is enabled, the Resolution option will not be displayed.
Well, I learnt that
__experimentalImageSizeControlis no longer deprecated: #56414
Which is the correct component to use?
I think it's fine as is. The __experimentalImageSizeControl component might be deprecated again in the future and a warning may be issued. Instead, it may make more sense to unlock and use the ResolutionTool component that is explicitly marked as private.
patil-vipul
pushed a commit
to patil-vipul/gutenberg
that referenced
this pull request
Jun 17, 2024
…ith ResolutionTool (WordPress#57540) Media & Text: Replace __experimentalImageSizeControl with ResolutionTool. Use the ToolsPanel and ToolsPanelItems, allowing settings to be reset. Co-authored-by: carolinan <[email protected]> Co-authored-by: t-hamano <[email protected]> Co-authored-by: richtabor <[email protected]>
2 tasks
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?
ImageSizeControl (__experimentalImageSizeControl) is used in the Media & text block. It was marked as deprecated in #51545.
This PR replaces __experimentalImageSizeControl with the ResolutionTool.
Partial for #52223
Why?
Deprecated controls should not be used.
How?
Adds a new tools panel and moves the existing block sidebar options to this panel.
Replaces __experimentalImageSizeControl with the ResolutionTool.
Testing Instructions
Testing Instructions for Keyboard
Deactivate Gutenberg.
Add two new media and text blocks with images. One with default options, one with the "Crop image to fill entire column" option enabled and a non default resolution.
Save.
Activate Gutenberg with the PR applied. Refresh the page and confirm that:
Save and view the front of the website. Confirm that the blocks display correctly, and the correct image size is loaded.
Screenshots or screencast
Before:
After:
