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

Fix the text editor styles after the mixins reset #14761

Merged
merged 3 commits into from
Apr 2, 2019

Conversation

youknowriad
Copy link
Contributor

Related #14509 (comment)

This turns out to be a little bit more complex that I originally thought.
I think it just surfaces the point I raised in #14509 saying that we should try to kill all "resets" in the future in favor of styles per components.

The problem is that the refactoring in #14509 applied the textarea and input resets more globally which means we need to reset these styles in the textarea of the text editor. The problem is that the specificity is higher for the reset because we can't apply it globally to the page (admin menus...), this uses !important to override these styles.

@youknowriad youknowriad added the [Type] Bug An existing feature does not function as intended label Apr 2, 2019
@youknowriad youknowriad added this to the 5.4 (Gutenberg) milestone Apr 2, 2019
@youknowriad youknowriad self-assigned this Apr 2, 2019
@jasmussen
Copy link
Contributor

I think it's okay to use the !importants here, and I agree with your ultimate goal. The CSS bleed from the rest of the wp-admin styles is also the underlying root cause behind #14407.

However I'm seeing this:

Screenshot 2019-04-02 at 10 35 06

It used to be a code font, and the font size appears too small. Maybe those need !important's too? And the focus style for the input field, did it used to be darker? And should the text editor even have the new left-border-heavy styles that blocks have? @kjellr

If this is an urgent PR, it is okay with me to ship this once the font and font size has been fixed. 👍 👍 — but then we need to look at the border-color/thickness separately.

Thanks Riad.

Copy link
Contributor

@kjellr kjellr left a comment

Choose a reason for hiding this comment

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

And should the text editor even have the new left-border-heavy styles that blocks have? @kjellr

Probably not. It looks like it's applied to just the page title. I can remove that in a separate PR. 👍

@youknowriad I left one tiny note — aside from that this is good to go.

packages/editor/src/components/post-text-editor/style.scss Outdated Show resolved Hide resolved
@youknowriad youknowriad merged commit 9af3f42 into master Apr 2, 2019
@aduth aduth deleted the fix/text-editor-styles branch April 2, 2019 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants