Skip to content

Conversation

@nightnei
Copy link
Contributor

@nightnei nightnei commented Dec 2, 2025

Proposed Changes

See comments in the PR below

Testing Instructions

PR's build should be green

@nightnei nightnei self-assigned this Dec 2, 2025
};
}

async closeWhatsNew() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved closeWhatsNew to onboarding to avoid duplications and keep consistency

return this.locator.getByRole( 'button', { name: 'Close' } );
}

async close() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need it

await expect( dialog.locator ).toBeVisible();
return dialog;
if ( ! isButtonVisible ) {
throw new Error( 'Add Site button not found on the main sidebar' );
Copy link
Contributor Author

@nightnei nightnei Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better to keep this function more specific, since if it's not available in UI then we can mute an error

Also, with the current use case - there is no such thing as "sidebar" inside onboarding, so it's a bit misleading to create MainSidebar and running openAddSiteModal, instead of directly running new AddSiteModal.

@nightnei nightnei requested a review from a team December 2, 2025 13:19
@github-actions
Copy link
Contributor

github-actions bot commented Dec 2, 2025

📊 Performance Test Results

Comparing c9ed3d0 vs trunk

site-editor

Metric trunk c9ed3d0 Diff Change
load 9141.00 ms 9339.00 ms +198.00 ms 🔴 2.2%

site-startup

Metric trunk c9ed3d0 Diff Change
siteCreation 14462.00 ms 14140.00 ms -322.00 ms 🟢 -2.2%
siteStartup 5929.00 ms 8031.00 ms +2102.00 ms 🔴 35.5%

Results are median values from multiple test runs.

Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants