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

Make auto-generated wp-block* classes consistent #1599

Merged
merged 1 commit into from
Jul 5, 2017

Conversation

nylen
Copy link
Member

@nylen nylen commented Jun 30, 2017

Follow-up to #1381 that modifies the generated classnames for blocks to improve consistency.

Mainly I don't think we should drop the core namespace from generated classnames for core blocks. From docs/design.md:

Key Values
All blocks are created equal, ...

From #1381 (comment):

This special handling is a definitive way in which core blocks are not equal to plugin blocks; it imposes additional cognitive load; and anything related to how we handle block names should be super simple, consistent, and explicit.

Also, the current structure breaks down a bit if, for example, a plugin registers a block named text/poetry. Then you end up with classes wp-block-text for core/text and wp-block-text-poetry for text/poetry.

The last bit is probably more an issue if someone releases a plugin whose name overlaps with one of our embeds, then the class names will get a bit confusing. We can fix this by making it really obvious how a block name maps to a class name, and making the different parts of the class name really clear.

Here are some example class names before and after this PR:

Block name Before After
core/text wp-block-text wp-block__core__text
core/image wp-block-image wp-block__core__image
core-embed/collegehumor wp-block-core-embed-collegehumor wp-block__core-embed__collegehumor
myplugin/someblock wp-block-myplugin-someblock wp-block__myplugin__someblock

As a first try, I've used __ because then the parts of the classname are completely unambiguous.

@nylen nylen requested review from youknowriad and mtias June 30, 2017 09:41
@nylen nylen force-pushed the update/wp-block-class-consistency branch from 2a5f6a1 to 46d710c Compare June 30, 2017 10:44
@mtias
Copy link
Member

mtias commented Jul 3, 2017

This is a bit different than namespacing on registration. I don't think "core" is providing any semantic value to the classes, mostly noise to most people styling things, which should be the primary concern here. The example of "text/poetry" doesn't seem great because such a namespace would have issues in many areas (PHP functions being text_*).

@nylen
Copy link
Member Author

nylen commented Jul 5, 2017

There are still a couple of things to fix here:

  • wp-block-core-embed-collegehumor - we should remove core here as well to be consistent
  • We should ensure that this generated classname appears first in the serialized markup

@nylen nylen force-pushed the update/wp-block-class-consistency branch from 29c6b53 to 1d2e94d Compare July 5, 2017 13:50
@nylen
Copy link
Member Author

nylen commented Jul 5, 2017

Previously we were using kebabCase here which changes strings such as table2 to table-2. This is not desirable. This PR also contains the fix + tests:

-<table class="wp-block-table-2">
+<table class="wp-block-table2">

@nylen
Copy link
Member Author

nylen commented Jul 5, 2017

The changes here are now much less extensive:

Block name Before After
core/table2 wp-block-table-2 wp-block-table2
core/image wp-block-image (no change)
core-embed/collegehumor wp-block-core-embed-collegehumor wp-block-embed-collegehumor
myplugin/someblock wp-block-myplugin-someblock (no change)

Merging to address the inconsistencies in #1599 (comment) and the bug in #1599 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants