-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Font Library: Fix modal scrollbar #60641
Conversation
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. |
@@ -181,7 +181,6 @@ function InstalledFonts() { | |||
) ) } | |||
</> | |||
) } | |||
<Spacer margin={ 16 } /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this was added to create some space between the content and the footer. I don't think this is needed now that it's possible to see the bottom of the content above the footer.
Size Change: +14 B (0%) Total Size: 1.75 MB
ℹ️ View Unchanged
|
packages/edit-site/src/components/global-styles/font-library-modal/style.scss
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I can see the bottom of the font library modal scrollbar now! Tested desktop and mobile screen sizes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure why you're seeing 68.8px there, although I know that's what the height was before I set it to 70px. Could this maybe be caused by a caching issue on the CSS?
Sorry, I was debugging with trunk 😅 This PR does indeed have a footer height of 70px and is working as expected.
I added the "Backport to WP Minor Release" tag to include it in WordPress 6.5.3. |
Thanks @matiasbenedetto! I've added this to the 6.5.x board for consideration in the 6.5.3 release. |
Confirming that this will be included in the 6.5.3 release. |
* Make footer a fixed height; add margin to modal content * Remove Spacer from bottom of Library tab content * Add variable for footer height size Co-authored-by: mikachan <[email protected]> Co-authored-by: creativecoder <[email protected]> Co-authored-by: t-hamano <[email protected]> Co-authored-by: jordesign <[email protected]> Co-authored-by: matiasbenedetto <[email protected]>
I just cherry-picked this PR to the update/fixes-for-wp-6-5-3 branch to get it included in the next release: 25c18ac |
* Make footer a fixed height; add margin to modal content * Remove Spacer from bottom of Library tab content * Add variable for footer height size Co-authored-by: mikachan <[email protected]> Co-authored-by: creativecoder <[email protected]> Co-authored-by: t-hamano <[email protected]> Co-authored-by: jordesign <[email protected]> Co-authored-by: matiasbenedetto <[email protected]>
What?
Improves the display of the Font Library modal's scrollbar to prevent it from disappearing behind the footer.
Why?
Fixes #54401.
How?
This adjusts the CSS of the Font Library modal's content to add a bottom margin, which means it can take into account the height of the footer when displaying the scrollbar. I've also made the footer a fixed height so that these two measurements always match.
Testing Instructions
Screenshots or screencast
Screen.Recording.2024-04-10.at.20.19.41.mov