Migrate from TextControl to InputControl to remove margin overrides #47161
Conversation
This file contains hidden or 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
|
Size Change: -144 B (0%) Total Size: 1.33 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in b9a817e. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/3917779508
|
49 tasks
mirka
reviewed
Jan 18, 2023
There was a problem hiding this comment.
Ah, this is a hard one 😅 (Related: #41961)
To share some context:
Placeholderitself has some very shaky CSS foundations. So this is a can of worms to start with.- We will be retiring the
__next36pxDefaultSizeprop (#46741). But that's ok, we can still use it here for the time being as a clean fix. We'll need to revisit these blocks when transitioning to the new size scheme anyway. - As for the wrapping behavior, we need to think about consistency across the Block Library, which has other blocks with this kind of responsive input & button wrapping (
core/embedorcore/table).
tl;dr — I think we should keep the wrapping behavior, so this PR can just remain a clean bug fix and not an entry point into a Placeholder overhaul rabbit hole 🙃 We can still do it cleanly with the HStack by enabling the wrap prop, and setting a min-width: 80% or something like that to the InputControl.
What do you think?
| <Button variant="primary" type="submit"> | ||
| { __( 'Use URL' ) } | ||
| </Button> | ||
| <HStack align="baseline"> |
|
Thank you for the context! This is a good lesson for me; I need to search GH a bit more thoroughly before I go down these rabbit holes... but I figured if it wasn't something we could do, I would at least learn from it! 😅
This is a great idea! I guess this whole PR is a bit of an odd in-between step, but hopefully, it is helpful to have the small fix in the meantime. |
b9a817e to
6696644
Compare
mirka
approved these changes
Jan 19, 2023
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?
Updated
TextControltoInputControlcomponent for RSS block.Why?
While working on this project to remove margin overrides, I found that moving this to InputControl required less CSS. It is also the plan to replace TextControl with InputControl per the storybook docs.
How?
By adding InputControl and HStack to remove CSS overrides.
Testing Instructions
Additional Notes
Before the changes, if the screen size was smaller, the input wouldn't match the button size (fixed in this PR):
However, the button would also move below the input on smaller screen sizes. Should it be below like in trunk?