-
Notifications
You must be signed in to change notification settings - Fork 4.7k
Omit serializing block attributes that match any default attribute values #1910
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,6 +25,10 @@ registerBlockType( 'core/text', { | |
|
|
||
| category: 'common', | ||
|
|
||
| defaultAttributes: { | ||
| dropCap: false, | ||
| }, | ||
|
|
||
| className: false, | ||
|
|
||
| attributes: { | ||
|
|
@@ -109,7 +113,7 @@ registerBlockType( 'core/text', { | |
|
|
||
| save( { attributes } ) { | ||
| const { align, content, dropCap } = attributes; | ||
| const className = dropCap && 'has-drop-cap'; | ||
| const className = dropCap ? 'has-drop-cap' : null; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any reason to not just do this inline like
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No reason, other than it was using a |
||
|
|
||
| if ( ! align ) { | ||
| return <p className={ className }>{ content }</p>; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| <!-- wp:core/latest-posts {"postsToShow":5,"displayPostDate":false} /--> | ||
| <!-- wp:core/latest-posts {"postsToShow":5,"displayPostDate":true} /--> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| <!-- wp:core/latest-posts {"postsToShow":5,"displayPostDate":false,"layout":"list","columns":3} /--> | ||
| <!-- wp:core/latest-posts {"displayPostDate":true} /--> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,7 +9,8 @@ | |
| [ | ||
| "... like this one, which is separate from the above and right aligned." | ||
| ] | ||
| ] | ||
| ], | ||
| "dropCap": false | ||
| } | ||
| } | ||
| ] | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need (or want) this to be a deep equality
isEqual, but rather just strict equality!==There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about when an attribute contain an object? Then this will fail
!==matching.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In other words,
value !== blockType.defaultAttributes[ key ]will always betruefor objects.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aduth 👍?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, not necessarily; not if the attribute value is never updated. But it's a fair point that if they are updated, and their nested structure matches the default value, it should probably be omitted all the same. I'm fine with
isEqual.