Conversation
2 tasks
|
Size Change: +27 B (0%) Total Size: 1.26 MB
ℹ️ View Unchanged
|
ciampo
approved these changes
Aug 1, 2022
Contributor
ciampo
left a comment
ciampo
left a comment
There was a problem hiding this comment.
Tested by using the Global CSS switcher:
- on
trunk, switching from "WordPress (common/forms)" to "Font only" would cause the component's font size to shift - on this PR, the font size stays the same when switching.
Just flagging that, relatively to typography, the line-height is also different (it can be easily observed in case of the Tooltip component when using the global CSS with the changes from this PR)
Member
Author
Mmm yeah, the |
noisysocks
reviewed
Aug 5, 2022
| import { Select } from '../../select-control/styles/select-control-styles'; | ||
|
|
||
| export const Wrapper = styled.div``; | ||
| export const Wrapper = styled.div` |
Member
There was a problem hiding this comment.
It doesn't look like this is used anywhere.
never mind I am a big idiot.
oandregal
changed the title
Tooltip (Experimental), CustomSelectControl, TimePicker: Add missing font sizes styles which were necessary in non-WordPress contexts
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Part of #42781
What?
Adds an explicit font size to components that didn't have them.
Why?
#42747 uncovered that some components rely on a
font-size: 13pxbeing set in the surrounding CSS context. This is not a given, and we need that font-size to be explicitly set in each component for them to be self-sufficient. (See #42781 for further reasoning.)How?
Add a
font-sizedeclaration to the highest-level wrapper possible, so inheritance can take care of the rest. This way we don't have to over-specify, and will cover any future additions to component internals.Testing Instructions
npm run storybook:devMake sure the global CSS injector is set to the "Font only" setting
Check these stories:
Screenshots or screencast