-
Notifications
You must be signed in to change notification settings - Fork 104
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
Add guidance for managing Performance feature plugins #1734
Add guidance for managing Performance feature plugins #1734
Conversation
Signed-off-by: Shyamsundar Gadde <[email protected]>
This comment was marked as resolved.
This comment was marked as resolved.
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
printf( | ||
/* translators: %1$s: opening anchor tag, %2$s: closing anchor tag */ | ||
esc_html__( 'Performance features are installed as plugins. To update features or remove them, %1$s manage them on the plugins screen. %2$s', 'performance-lab' ), | ||
'<a href="' . esc_url( admin_url( 'plugins.php?s=WordPress%20Performance%20Team&plugin_status=all' ) ) . '">', |
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.
Can we use add_query_arg()
here instead?
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.
Done in 8f670f5.
Signed-off-by: Shyamsundar Gadde <[email protected]>
What do you think about centering the text? Screen.recording.2024-12-12.09.39.01.webm |
+1 for the |
IMO, aligning the text center looks better: Screen.recording.2024-12-13.16.27.48.webmCompared with notice info: Screen.recording.2024-12-13.16.26.49.webm@adamsilverstein What do you think? |
I added the two options as suggestions. I can go with either, although I slightly prefer Option 1. Someone please accept the suggestion they think is better, and then we can merge this PR. |
This comment was marked as resolved.
This comment was marked as resolved.
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.
@westonruter @ShyamGadde Noticing one more small but important thing here that needs to be fixed before merge.
Co-authored-by: Felix Arntz <[email protected]>
Signed-off-by: Shyamsundar Gadde <[email protected]>
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 @ShyamGadde, looks great!
Summary
Fixes #1695
Relevant technical choices
Added an informative paragraph to the bottom of the Performance Features screen as seen in the screenshot below: