-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Add named colors, polish HTML editor #354
Conversation
- Normalization CSS - HTML Editor font - Toolbar height
editor/editor/style.scss
Outdated
|
||
* { | ||
box-sizing: border-box; | ||
} |
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.
This is already added here https://github.com/WordPress/gutenberg/blob/add/polish/editor/assets/stylesheets/main.scss#L19 and it's relative to the editor only to avoid altering any other WordPress style (sidebar etc...)
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.
Ah, good catch, still finding my way here.
editor/editor/style.scss
Outdated
@@ -97,3 +112,8 @@ | |||
} | |||
} | |||
} | |||
|
|||
/* Hide footer */ | |||
#wpfooter { |
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.
Should we move this to the global stylesheet main.scss
.
Should we add it under the .toplevel_page_gutenberg
like other WP style tweaks (to avoid altering these style in other pages, if ever the editor style is mixed with all the WPAdmin stylesheets) https://github.com/WordPress/gutenberg/blob/add/polish/editor/assets/stylesheets/main.scss#L5
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.
Yep. Perfect.
$editor-font: "Noto Serif", serif; | ||
$editor-html-font: "Meno", monospace; |
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.
Is this font loaded by default in WordPress? or should we load it in index.php
like the "Noto" font?
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 expect this font stack to grow further — Menlo is an OSX bundled system font. I will return with a better font stack later, but I don't think we should load any fonts separately.
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.
Is it "Menlo" or "Meno"?
So, you're saying we keep those as is temporary and we'll include them in the Repo later? I'm not sure I understand your point :P.
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.
Facepalm. It was Menlo. My apologies.
So, you're saying we keep those as is temporary and we'll include them in the Repo later?
Are you referring to a webfont or just a font stack that picks from fonts available on your system?
I expanded the monospaced font stack so it's not temporary. Or rather, it's temporary insofar as I expect that we will probably revisit this in the future as I feel like there's a history here. But no webfonts are being loaded — we are just using fonts that you are likely to have on your system.
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.
Nice improvements 🚢
Thanks Riad! |
This PR adds the named new suggested grays by @hugobaeta, as well as adds a nice font for the HTML editor.