Patterns: Add a title to the category delete flow and increase line height #59645
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
scruffian
added
[Type] Enhancement
scruffian
commented
Mar 6, 2024
| @@ -58,7 +58,7 @@ export default Object.assign( {}, CONTROL_PROPS, TOGGLE_GROUP_CONTROL_PROPS, { | |||
| fontSizeMobile: '15px', | |||
| fontSizeSmall: 'calc(0.92 * 13px)', | |||
| fontSizeXSmall: 'calc(0.75 * 13px)', | |||
| fontLineHeightBase: '1.2', | |||
| fontLineHeightBase: '1.4', | |||
There was a problem hiding this comment.
This feels like it could have significant impact.
There was a problem hiding this comment.
Why not add this as a specific css for the specific ConfirmDialog we're having a problem with?
There was a problem hiding this comment.
I could do but then it feels like patching over a problem instead of dealing with the root cause.
There was a problem hiding this comment.
I think this should be addressed as a separate change with before/after screenshots, and reviewed by the design team. It indeed will have a big impact and we should ensure it's intentional.
There was a problem hiding this comment.
By the way I think it's perfectly reasonable to take the targeted approach that @draganescu is suggesting to address the problem at hand. Fixing this config file is a pretty broad-scope task, potentially with back-compat considerations, so it should be triaged as a separate issue IMO. It shouldn't block this dialog PR.
|
Size Change: +18 B (0%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
|
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. |
tyxla
reviewed
Mar 18, 2024
| @@ -58,7 +58,7 @@ export default Object.assign( {}, CONTROL_PROPS, TOGGLE_GROUP_CONTROL_PROPS, { | |||
| fontSizeMobile: '15px', | |||
| fontSizeSmall: 'calc(0.92 * 13px)', | |||
| fontSizeXSmall: 'calc(0.75 * 13px)', | |||
| fontLineHeightBase: '1.2', | |||
| fontLineHeightBase: '1.4', | |||
There was a problem hiding this comment.
I think this should be addressed as a separate change with before/after screenshots, and reviewed by the design team. It indeed will have a big impact and we should ensure it's intentional.
packages/edit-site/src/components/page-patterns/delete-category-menu-item.js
Outdated
Show resolved
Hide resolved
|
Thanks for the reviews. I have updated this PR now. |
mirka
added
[Package] Edit Site
carstingaxion
pushed a commit
to carstingaxion/gutenberg
that referenced
this pull request
Mar 27, 2024
…eight (WordPress#59645) * Add a title to the category delete flow and increase line height * reset line height * update the title text Co-authored-by: scruffian <[email protected]> Co-authored-by: draganescu <[email protected]> Co-authored-by: tyxla <[email protected]> Co-authored-by: jasmussen <[email protected]> Co-authored-by: mirka <[email protected]> Co-authored-by: t-hamano <[email protected]> Co-authored-by: talldan <[email protected]> Co-authored-by: jameskoster <[email protected]> Co-authored-by: richtabor <[email protected]> Co-authored-by: getdave <[email protected]> Co-authored-by: youknowriad <[email protected]> Co-authored-by: annezazu <[email protected]>
22 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?
Partially fixes #58690 by aligning the look of the two dialogs.
Why?
Consistency.
How?
Testing Instructions
Testing Instructions for Keyboard
This is a visual change
Screenshots or screencast