💦 FULL SET: WordPress/performance/pull/ - Uncensored 2025
-
Notifications
You must be signed in to change notification settings - Fork 139
Conversation
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 @westonruter, this looks great so far. I left some feedback, most importantly I'm thinking we could display the field in a slightly more prominent way.
|
@westonruter Given that this applies to the newly added screen, I've added this to the 2.6.0 milestone and updated the base branch accordingly, let's try to merge it in time. |
db0ee21 to
671c001
Compare
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 Some mostly very minor follow-up feedback.
…n translation strings
Co-authored-by: Felix Arntz <[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.
This is almost there. A few last points of feedback.
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 One final nit-pick on wording, but otherwise this looks great! 🎉
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 @westonruter for the PR. LGTM! I left some non blocking feedback.
|
@westonruter Could you take a look at @mukeshpanchal27's feedback please so that we can try to merge this today? |
Co-authored-by: Mukesh Panchal <[email protected]>
|
@westonruter @mukeshpanchal27 In light of the CSS feedback, after taking a closer look I've pushed an update in 7e8c9c5 that changes the markup to match what is used on WP core's own settings screens, specifically for the |
Summary
Implements output buffering checkbox as discussed at #784 (comment).
Relevant technical choices
A new boolean
output_bufferingarray item is added to theperflab_server_timing_settingsoption. This value, defaulting tofalse, will be used if there is noperflab_server_timing_use_output_bufferfilter present.Screenshots
When the option is not enabled, the note at the top of the screen remains:
When the option is enabled, the note at the top is removed:
When a
perflab_server_timing_use_output_bufferfilter is added which returnstrue, the checkbox is checked and disabled with a note about the filter being present:The same is true when the filter returns
false:Checklist
[Focus]orInfrastructurelabel.[Type]label.no milestonelabel.