-
Notifications
You must be signed in to change notification settings - Fork 40
Ensure plugin detail sections are ordered consistently #283
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
Ensure plugin detail sections are ordered consistently #283
Conversation
Signed-off-by: meszarosrob <hmm@meszarosrob.com>
Signed-off-by: meszarosrob <hmm@meszarosrob.com>
Signed-off-by: meszarosrob <hmm@meszarosrob.com>
Signed-off-by: meszarosrob <hmm@meszarosrob.com>
… namespace Signed-off-by: meszarosrob <hmm@meszarosrob.com>
Signed-off-by: meszarosrob <hmm@meszarosrob.com>
Signed-off-by: meszarosrob <hmm@meszarosrob.com>
costdev
left a comment
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.
Thanks a ton for the PR and the tests @meszarosrob !
I've left a totally optional suggestion as well as some change requests for consistency within the plugin.
Signed-off-by: meszarosrob <hmm@meszarosrob.com>
Signed-off-by: meszarosrob <hmm@meszarosrob.com>
Signed-off-by: meszarosrob <hmm@meszarosrob.com>
Signed-off-by: meszarosrob <hmm@meszarosrob.com>
|
@costdev Can you please re-review? |
costdev
left a comment
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.
Thanks for the updates @meszarosrob!
| 'faq', | ||
| 'screenshots', | ||
| 'changelog', | ||
| 'security', |
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.
security is not a standard section and would therefore be grouped in other_notes
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.
It's not something that is returned/used by WP.org; however, it is defined in the protocol https://github.com/fairpm/fair-protocol/blob/main/specification.md#sections, so I have included it.
My assumption (which might be totally wrong) was that if it's in the protocol, then eventually it might get used.
If somebody passes it today, it will be displayed in the tabs.
|
There's actually an easier way to accomplish this. It works for both FAIR and dot org. add_filter('plugins_api_result', 'sort_sections_in_api', 15, 1);
function sort_sections_in_api($result) {
// Order sections.
$ordered_sections = array(
'description',
'installation',
'faq',
'screenshots',
'changelog',
'upgrade_notice',
'other_notes',
);
$properly_ordered = array_merge( array_fill_keys( $ordered_sections, '' ), $result->sections );
$result->sections = array_filter($properly_ordered);
return $result;
}Thanks @costdev for the tweaks. |
This PR reorders the sections in the plugin details modal to match the WP.org order to resolve #279.
The ordering is applied in two places since plugin information is retrieved in two distinct ways, from what I can see.
The section order differs between
api.wordpress.organdapi.aspirecloud.net, and while this could be resolved on the AspireCloud side, the default repo domain can be changed, so I chose to "normalize" the returned details.In the other instance, since no filter is currently available when
MetadataDocumentis returned, and given the considerable changes planned in #284, I've implemented the ordering at the UI level for now.I've added basic tests for the ordering, but since there was only a sample test file available, this might not match what you had in mind.
Feel free to modify or close this PR without any problems if it doesn't align with the project direction.