-
Notifications
You must be signed in to change notification settings - Fork 4.7k
Add typing test when top toolbar is mounted #56908
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
Conversation
Ideally, this metric should be similar to the performance of the typing metric of when the toolbar is not mounted (typing perf results).
|
Size Change: 0 B Total Size: 1.71 MB ℹ️ View Unchanged
|
|
Flaky tests detected in 98e57e0. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7143932144
|
draganescu
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.
I think this is a good addition as long as typing requires us to unmount the toolbar b/c of performance.
| minWithTopToolbarType: minimum( results.typeWithTopToolbar ), | ||
| maxWithTopToolbarType: maximum( results.typeWithTopToolbar ), |
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.
Let's skip those. We didn't include them with the recently added typeWithToolbar as well.
|
|
||
| test( 'Setup the test post', async ( { admin, perfUtils, editor } ) => { | ||
| await admin.createNewPost(); | ||
| await editor.setIsFixedToolbar( true ); |
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 a global, persistent setting so it needs to be reverted after the test or it will affect other metrics. See this PR as an example on how to handle it!
Ideally, this metric should be similar to the performance of the typing metric of when the toolbar is not mounted (typing perf results). This gives us a decent proxy for how much the
<BlockToolbar />is doing on each keypress. At the moment, there seems to be a lot of unnecessary rendering happening on each keypress, especially with<BlockSwitcher />.What?
Adds a performance test for typing with the top toolbar.
Why?
To measure performance of typing without the toolbar mounted (default typing metric) vs mounted (top toolbar).
How?
Duplicates the typing test, but sets the Top Toolbar option.
Testing Instructions
Performance Tests for this PR should pass. Check for
typeWithTopToolbar.