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

Style nested `<ul>`s with circles instead of normal bullets #10358

Merged

Conversation

@chrisvanpatten
Copy link
Member

@chrisvanpatten chrisvanpatten commented Oct 5, 2018

Discovered while working on #10303 that we style nested unordered lists with a normal bullet, instead of circle which is more typical of user agent stylesheets (aka default browser styles).

In the Chrome user agent styles, for instance, you can see the default is overridden:

devtools_-dev_tomodomo_co_wp-admin_post_php_post_78_action_edit_and_edit_page tomodomo _wordpress

This adds a rule to the editor stylesheets to handle nested styles.

Here's the before:

edit_page_ tomodomo _wordpress

And here's the after:

edit_page_ tomodomo _wordpress

One small implementation note: I've removed a :not() selector that was previously ensuring the <ul> CSS rule did not apply to .wp-block-gallery and instead placed an override in the gallery block's editor stylesheet.

I expect this to be slightly controversial, but this is intentional to avoid a selector specificity issue. Basically, ul:not(.wp-block-gallery) is more specific than ol ul. We would have had to do something like ol:not(.wp-block-gallery) ul or whatever to increase the specificity which is kind of annoying and unnecessary. To me, this feels like a case where the gallery should manage its own overrides.

@chrisvanpatten
Copy link
Member Author

@chrisvanpatten chrisvanpatten commented Oct 5, 2018

(As it so often is, the Travis failure seems unrelated.)

Copy link
Member

@tofumatt tofumatt left a comment

This makes a lot more sense to me code-wise. The editor styles specifically :not() targeting a single block's styles rather than that block managing it seems hacky/inelegant. I like this solution way more.

I haven't tested this locally but I prefer this approach. 👍 on the code aspect.

@chrisvanpatten
Copy link
Member Author

@chrisvanpatten chrisvanpatten commented Oct 5, 2018

Thanks @tofumatt! I'll still wait for an "OK" from a design person before merging since this is technically a visual change :)

@chrisvanpatten
Copy link
Member Author

@chrisvanpatten chrisvanpatten commented Oct 8, 2018

@jasmussen Sorry to ping just want to see if I can get a visual OK on this so I can get it in today :)

@jasmussen
Copy link
Contributor

@jasmussen jasmussen commented Oct 8, 2018

Apologies for missing this.

I think this is good to go. No strong opinion, but also no objections. 👍 👍

@chrisvanpatten
Copy link
Member Author

@chrisvanpatten chrisvanpatten commented Oct 8, 2018

Thanks @jasmussen and no worries for missing it! ^__^

@chrisvanpatten chrisvanpatten merged commit 69ced17 into WordPress:master Oct 8, 2018
2 checks passed
2 checks passed
codecov/project 49.32% remains the same compared to 3770364
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@chrisvanpatten chrisvanpatten deleted the TomodomoCo:feature/nested-list-bullets branch Oct 8, 2018
@tofumatt tofumatt added this to the 4.0 milestone Oct 8, 2018
nickcernis added a commit to studiopress/genesis-sample that referenced this pull request Oct 12, 2018
Matches new Gutenberg 4.0rc1 editor styling, and default browser styles.

See WordPress/gutenberg#10358.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.