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

Change attribute serialization to explicit opt-in #988

Closed
wants to merge 1 commit into from

Conversation

aduth
Copy link
Member

@aduth aduth commented Jun 1, 2017

Related: #865, #391

This pull request seeks to change attribute serialization behavior, currently inferred as the difference between a block's actual attributes at time of serialization and the attributes definition. With these changes, it will instead be required that a block explicitly control its comment encoding behavior by returning an object from an encodeAttributes function. This is implemented exactly as documented from the registerBlockType documentation.

Testing instructions:

Ensure that tests pass:

npm test

Verify that block comment serialization continues to behave as expected for Button align, Image align, and Quote style blocks, when changing these values and switching to Text mode.

@aduth aduth added the [Feature] Block API API that allows to express the block paradigm. label Jun 1, 2017
encodeAttributes( attributes ) {
const { align } = attributes;
return { align };
},
Copy link
Contributor

Choose a reason for hiding this comment

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

don't you think the metadata( 'align' ) is better for consistency? It's just another way of parsing the attribute.

Copy link
Member Author

Choose a reason for hiding this comment

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

don't you think the metadata( 'align' ) is better for consistency? It's just another way of parsing the attribute.

I suppose it depends where we take attributes initialization. I tried to communicate my current state of mind earlier at #391 (comment) . If matching included encoded attributes, I'd agree we wouldn't need this function at all. I might try to spend some time tomorrow exploring AST selectors like the ESLint ones I mentioned in that comment. If those work out, I don't think we'd really want to use them for comment sourcing.

@aduth
Copy link
Member Author

aduth commented Jul 17, 2017

Closing in favor of #1905

@aduth aduth closed this Jul 17, 2017
@aduth aduth deleted the add/encode-attributes branch July 17, 2017 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Block API API that allows to express the block paradigm.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants