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

Alignment: setting alignment attributes should be consistent across blocks #7909

Closed
designsimply opened this issue Jul 11, 2018 · 3 comments
Closed
Labels
[Feature] Blocks Overall functionality of blocks [Priority] Low Used to indicate that the issue at hand isn't a top priority to address and can be handled later [Type] Code Quality Issues or PRs that relate to code quality [Type] Enhancement A suggestion for improvement.

Comments

@designsimply
Copy link
Member

designsimply commented Jul 11, 2018

Split out from #4010 where @bradyvercher says:

Toolbar onChange Callback

Some blocks define a class method (requires binding in the constructor), others define a local function, and some use an inline arrow function. They all do the same thing. I'd like to see this be more consistent across blocks.

Considering how simple it is, I think using an inline arrow function would make things more concise and easier to read:

<BlockAlignmentToolbar
    value={ align }
    onChange={ value => setAttributes({ align: value }) }
/>
@designsimply designsimply added [Type] Enhancement A suggestion for improvement. [Feature] Blocks Overall functionality of blocks Needs Decision Needs a decision to be actionable or relevant labels Jul 11, 2018
@karmatosed karmatosed removed the Needs Decision Needs a decision to be actionable or relevant label Jul 12, 2018
@karmatosed
Copy link
Member

Both alignment used and classes should be consistent across blocks in my opinion. This is similar to the visual side in #3785.

@timhibberd
Copy link

@designsimply - I am championing an issue that I believe is related to this. If you could spare a moment and have a look and comment that would be fantastic. Regardless of how you feel about the issue I raised, I agree completely with this issue you raised...block styling consistency is a must if external themes are going to play well with GB. Thanks in advance...much appreciated :-)

@aduth aduth added the [Priority] Low Used to indicate that the issue at hand isn't a top priority to address and can be handled later label Apr 22, 2019
@youknowriad
Copy link
Contributor

Closing this as it's not really important and we'd like to bring consistency with #20650

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Blocks Overall functionality of blocks [Priority] Low Used to indicate that the issue at hand isn't a top priority to address and can be handled later [Type] Code Quality Issues or PRs that relate to code quality [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

5 participants