Migrate screen via the site import link #543
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Migration screens
ajayadav09
commented
Apr 17, 2024
ajayadav09
added
Code Review
arunshenoy99
requested changes
Apr 25, 2024
There was a problem hiding this comment.
- We need to group imports in all files based on our discussion. Example
- We also have a migrate button in the default flow that uses the migration URL. Should we make both of these behaviors consistent, or should we just remove the button from the default flow and keep it only on the fork step?
| @@ -0,0 +1,85 @@ | |||
| import { useEffect } from '@wordpress/element'; | |||
There was a problem hiding this comment.
Shall we reuse the existing error state? This one seems to be almost similar to the other one, and it does not handle mobile view, if I am not mistaken.
There was a problem hiding this comment.
It handles mobile view, I will check how can re-use the existing error state for migration as well.
src/OnboardingSPA/components/NewfoldInterfaceSkeleton/SiteBuild/index.js
Outdated
Show resolved
Hide resolved
|
|
||
| const loadData = async () => { | ||
| const migrateUrl = await getSiteMigrateUrl(); | ||
| if ( migrateUrl?.body ) { |
There was a problem hiding this comment.
We need to check for response.error instead of body.
Co-authored-by: Arun Shenoy <[email protected]>
Co-authored-by: Arun Shenoy <[email protected]>
|
Ready for re-review @arunshenoy99 @officiallygod |
arunshenoy99
approved these changes
May 2, 2024
arunshenoy99
added
QA
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed changes
Create Migration step for screens
https://www.figma.com/file/XlfIRs6b4vNPPK71HLLgrJ/Token-Flow?type=design&node-id=1447-57683&mode=design&t=doVZxjNrxswvueKi-0
Type of Change
Checklist
Further comments