Rename list view prop expandNested to isExpanded
#40731
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
getdave
added
[Type] Code Quality
|
Size Change: -144 B (0%) Total Size: 1.23 MB
ℹ️ View Unchanged
|
ciampo
approved these changes
Apr 29, 2022
There was a problem hiding this comment.
The renaming makes sense to me!
Just a consideration — in the @wordpress/components package we would need to be more careful when renaming props (as they would introduce breaking changes), but I assume that in the block-editor and edit-site packages the rules are more relaxed?
Probably better to get a second pair of eyes on this to make sure :)
|
@ciampo Thanks for the review 👍 Totally agree about not modifying public APIs without a proper deprecation strategy. Currently listView is exposed as |
|
@talldan Any chance you would be the "second pair of eyes" on this one? |
talldan
reviewed
May 3, 2022
packages/edit-site/src/components/sidebar/navigation-menu-sidebar/navigation-menu.js
Outdated
Show resolved
Hide resolved
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Renames the
expandNestedprop of<ListView>toisExpanded.Why?
This was discussed at length in #39486 (comment) and it was decided to rename the prop.
How?
Do the prop renaming. Rename intermediate vars to take account of the resulting naming clash.
Testing Instructions
truegutenberg/packages/block-editor/src/components/list-view/index.js
Line 75 in dd7db32
Screenshots or screencast