-
Notifications
You must be signed in to change notification settings - Fork 8
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
fadein loading state for sitetype steps #147
fadein loading state for sitetype steps #147
Conversation
screen-capture.webm |
src/OnboardingSPA/pages/Steps/GetStarted/SiteTypeSetup/PrimarySite/index.js
Show resolved
Hide resolved
<NavCardButton | ||
text={ __( content.buttonText ) } | ||
disabled={ categoriesArray[ 0 ]?.subCategories === null } | ||
/> |
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.
We could include these tags within the StepStateHandler tag too
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.
we can .. but then the button will also not appear when the page loads .. it will appear after a delay of 500ms ... we will have to take a call on that whether we want to do that with the buttons also
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.
We can ask David but I don't think it's a good idea to move the NavCardButton
and the NeedHelpTag
into the handler because these are not dependent on the data that is being waited for and hence do not need the delay, if navigation is the concern then let me remind you that the user can always navigate using the header navigation regardless. We also do not want to hide the need help tag incase something breaks and the component does not fade in its children.
src/OnboardingSPA/pages/Steps/GetStarted/SiteTypeSetup/PrimarySite/index.js
Outdated
Show resolved
Hide resolved
src/OnboardingSPA/pages/Steps/GetStarted/SiteTypeSetup/SecondarySite/index.js
Outdated
Show resolved
Hide resolved
Also, made changes to PRESS 2-462 to use an Animate component, please do make the changes accordingly and do provide any feedback or changes that further need to be made. |
81a2c3d
to
6c85151
Compare
No description provided.