Skip to content
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

back button hidden on Editor step #593

Merged
merged 5 commits into from
Jul 24, 2024

Conversation

ajayadav09
Copy link
Contributor

@ajayadav09 ajayadav09 commented Jul 4, 2024

Proposed changes

JIRA: https://jira.newfold.com/browse/PRESS0-1551

  • Hide the navigation bar on preview step
  • Hide the back button on Editor step

Type of Change

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Update (if none of the other choices apply)

Checklist

  • I have read the CONTRIBUTING doc
  • Linting and tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Further comments

@ajayadav09
Copy link
Contributor Author

[PRESS0-1551] (https://jira.newfold.com/browse/PRESS0-1551)

@ajayadav09 ajayadav09 requested a review from officiallygod July 4, 2024 13:40
@officiallygod officiallygod added the Code Review The PR is in Code Review label Jul 5, 2024
Copy link
Member

@officiallygod officiallygod left a comment

Choose a reason for hiding this comment

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

Do we also remove the whole area where we used to have the back button? also cause of that the alignment of the previews seem off and more closer to the top.

Screenshot 2024-07-08 at 8 48 14 AM

Comment on lines -61 to -67
<Back
path={
isPreviewStep
? stepSiteGenSiteLogo.path
: previousStep.path
}
/>
Copy link
Member

@officiallygod officiallygod Jul 8, 2024

Choose a reason for hiding this comment

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

I think why we included this was when a user does a back on Preview page he ends up on the same page again as the experience redirects him back to this page and he can never go back, But now I am unsure if a user can ever go and change the prompt again, Is that requirement fine?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we are removing the navigation bar itself from the preview step. the check for the isPreviewStep is not relevant anymore on the back button. The behaviour to stay redirect back on the preview step is still the same.

Copy link
Member

Choose a reason for hiding this comment

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

Let's confirm if it's OK to hide the header for that step.

Copy link
Member

Choose a reason for hiding this comment

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

@officiallygod, @ajayadav09 has confirmed that we have a go ahead.

@ajayadav09
Copy link
Contributor Author

Do we also remove the whole area where we used to have the back button? also cause of that the alignment of the previews seem off and more closer to the top.

Screenshot 2024-07-08 at 8 48 14 AM

the navigation bar was empty so removed it and adjusted the space from the top:
Screenshot 2024-07-17 at 11 23 06 AM
Screenshot 2024-07-17 at 11 24 18 AM

@arunshenoy99 arunshenoy99 added QA This PR is in QA and removed Code Review The PR is in Code Review labels Jul 17, 2024
@ajayadav09 ajayadav09 added Ready to merge The Code Review and QA is done and it can be merged. and removed QA This PR is in QA labels Jul 23, 2024
@ajayadav09
Copy link
Contributor Author

PRESS0-1551

@officiallygod officiallygod merged commit 9a7b20e into trunk Jul 24, 2024
6 of 9 checks passed
@officiallygod officiallygod deleted the PRESS0-1551-back-button-visibility branch July 24, 2024 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready to merge The Code Review and QA is done and it can be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants