Skip to content
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

Assign default font and font size styles to the notice component #13817

Merged
merged 1 commit into from
Feb 11, 2019

Conversation

kjellr
Copy link
Contributor

@kjellr kjellr commented Feb 11, 2019

Our standard notices all appear using our default sans-serif system font stack, at 13px:

screen shot 2019-02-11 at 11 09 23 am

However, these font styles are all inherited. If a notice component is inserted into a custom block, its styles will usually be overridden by the theme's stylesheet:

screen shot 2019-02-11 at 11 05 43 am

Notices are a system-level alert, so their styles should not vary based on the user's theme. This PR adds basic font and font size rules to the Notice component to prevent that:

screen shot 2019-02-11 at 11 04 24 am

@@ -1,4 +1,6 @@
.components-notice {
font-family: $default-font;
font-size: $default-font-size;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense to me. I wonder if we should do an audit and do this consistently across our reusable UI components. For example, I also noticed that the block placeholders and the buttons can be altered by editor styles.

Also what properties should we add: font-family, font-size, maybe also color, background and line-height?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, definitely might be worth an audit. 👍 I'll merge this in for now though.

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regardless of the more "global" discussion, this LGTM 👍

@kjellr kjellr merged commit f0261ce into master Feb 11, 2019
@kjellr kjellr deleted the update/add-default-font-size-for-notice-component branch February 11, 2019 17:59
@kjellr kjellr added this to the 5.1 (Gutenberg) milestone Feb 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Design Feedback Needs general design feedback. [Package] Notices /packages/notices
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants