-
Notifications
You must be signed in to change notification settings - Fork 54
Refactor blueprint deeplink handling in the Add site modal
#2216
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
AddSiteProvider to handle states related to site creation modal
94d3f63 to
d1688eb
Compare
AddSiteProvider to handle states related to site creation modalAdd site modal
….tsx && git rebase --continue 2>&1 Refactor deeplink handling to use AddSiteProvider context
8553a4c to
0d040d0
Compare
📊 Performance Test ResultsComparing 007e9fc vs trunk site-editor
site-startup
Results are median values from multiple test runs. Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change |
sejas
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 cleaning the code and removing AddSiteContentWithDeeplinkSupport, now the logic is much clearer.
I tested creating the 4 types of sites as first site and as second sites, and I didn't see any regression. The issue STU-1121 is still present, but we'll tackle that in a separate PR.
gcsecsey
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 also tested this and found no regressions with either creating the sites or running the Blueprints via deeplinks.
Thank you for carrying out the refactor, Ivan! IMO this structure is much clearer.
Related issues
Proposed Changes
The goal of this PR is to streamline the handling of blueprint deeplinks in the Add site modal and improve maintainability. We are getting rid of 34 lines of code.
AddSiteContentWithDeeplinkSupportcomponentuseAddSiteTesting Instructions
npm install && npm start.or the ones in https://jsbin.com/levaburate/2/edit?html,css,output .
Pre-merge Checklist