Add media order normalization add tests for https://github.com/WordPress/wordpress-develop/pull/10680 #10682
Conversation
Add order normalization in the Attachments model initialize method to ensure the 'order' property is always 'ASC' or 'DESC' (defaulting to 'DESC' for invalid values). This provides consistent behavior across all attachment collections. Part of fixing Media Library Grid view ordering when order query var is not normalized. See #64467.
Since Query inherits from Attachments, and Attachments now normalizes the order property in its initialize method, remove the duplicate normalization code from Query.get() to avoid redundancy and ensure consistent behavior. Part of fixing Media Library Grid view ordering when order query var is not normalized. See #64467.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
mukeshpanchal27
left a comment
mukeshpanchal27
left a comment
There was a problem hiding this comment.
Thanks for the PR. Left some suggestion.
| this.props.on( 'change:query', this._changeQuery, this ); | ||
|
|
||
| this.props.set( _.defaults( options.props || {} ) ); | ||
| options.props = _.defaults( options.props || {} ); |
There was a problem hiding this comment.
_.defaults() only adds properties from the source object if they don't exist in the target. Since options.props || {} creates a fresh object. Direct assignment is cleaner.
| options.props.order = options.props.order.toUpperCase(); | ||
| if ( 'ASC' !== options.props.order && 'DESC' !== options.props.order ) { | ||
| options.props.order = 'DESC'; | ||
| } |
There was a problem hiding this comment.
| options.props.order = options.props.order.toUpperCase(); | |
| if ( 'ASC' !== options.props.order && 'DESC' !== options.props.order ) { | |
| options.props.order = 'DESC'; | |
| } | |
| const normalizedOrder = options.props.order.toUpperCase(); | |
| options.props.order = ( 'ASC' === normalizedOrder || 'DESC' === normalizedOrder ) | |
| ? normalizedOrder | |
| : 'DESC'; |
Simplified
3 participants
No description provided.