-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
List View: Return primitive value for 'hideInserter' in Appender component #52161
Conversation
Size Change: -11 B (0%) Total Size: 1.44 MB
ℹ️ View Unchanged
|
Flaky tests detected in 98dbca4. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5420800508
|
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.
LGTM 👍 🚀
Thanks, @tyxla! Do you think it makes sense to return early in Something like this: export default function isShallowEqual( a, b ) {
if ( a === b ) {
return true;
}
if ( a && b ) {
if ( a.constructor === Object && b.constructor === Object ) {
return isShallowEqualObjects( a, b );
} else if ( Array.isArray( a ) && Array.isArray( b ) ) {
return isShallowEqualArrays( a, b );
}
}
return a === b;
} cc @jsnajdr |
I think it's a neat idea, especially given that we're doing it in all code paths (both for arrays and objects). It could help reduce some duplication at no cost. Just note that it's will no longer make much sense to have Feel free to open a PR, I'm happy to review it 👍 |
We'll have to leave checks in other code paths as they're exported and can be used separately. |
Good point, that's correct. In that case, there isn't that much of an improvement if we do it one more time earlier than the condition IMHO. |
Thanks for sharing your thoughts, @tyxla! |
What?
PR updates the selector to return the
boolean
value directory.Why?
The "shallow equality" check will skip extra steps for primitive values.
P.S. I was experimenting if we can make a screen reader announcement without
useEffect
and decided to push this minor improvement.Testing Instructions
Confirm that the list view Appender is correctly rendered for the Navigation block.
Screenshots or screencast