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

Add button class to button output for styling #1354

Closed
wants to merge 3 commits into from
Closed

Add button class to button output for styling #1354

wants to merge 3 commits into from

Conversation

jdevalk
Copy link
Contributor

@jdevalk jdevalk commented Jun 22, 2017

Without a button class, styling this element on the frontend is hard :)

Without a `button` class, styling this element on the frontend is hard :)
@@ -80,7 +80,7 @@ registerBlockType( 'core/button', {
const { url, text, title, align = 'none' } = attributes;

return (
<div className={ `align${ align }` }>
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest blocks-button instead to avoid ambiguity with already defined button classes.

Copy link
Member

Choose a reason for hiding this comment

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

We should take a step back and figure out a consistent way for naming these classes across all blocks. Not sure blocks-{name} will be enough. Maybe we should use the namespace of the block wp-core-button? This is something we want to get right and not change.

Copy link
Member

Choose a reason for hiding this comment

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

All blocks have a unique attribute like: div[data-type="core/preformatted"].

@mtias
Copy link
Member

mtias commented Jun 23, 2017

This is superceded by #1381 which adds auto-generated wrapper classes to most blocks.

@gziolo
Copy link
Member

gziolo commented Sep 22, 2017

It's fixed now with #1381. All block have wp-block-button as of this change. Closing this one.

@gziolo gziolo closed this Sep 22, 2017
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.

5 participants