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

feat!: Remove beta gate, deprecate MVP site #565

Merged
merged 13 commits into from
Mar 23, 2022
Merged

Conversation

suzubara
Copy link
Contributor

@suzubara suzubara commented Mar 16, 2022

SC-145

Proposed changes

This PR removes the opt-in beta UX from the portal, and deprecates the MVP routes by redirecting them to the 404 page (deprecation will be completed in sc-146 when the MVP code is removed altogether). The beta banner is also removed from the layout, so users can no longer join or leave beta.

This change also revealed an existing bug with the canonical link tag, I created a follow up story for that: sc-386

  • add link to officer PDF?

Reviewer notes

  • Log into the portal and confirm that you can only access the new portal (previously marked as beta), there are no more UI elements referring to beta, and you cannot access the MVP pages anymore.

Code review steps

As the original developer, I have

  • Met the acceptance criteria, or will meet them in subsequent PRs or stories
  • Created/modified automated unit tests in Jest
    • Including jest-axe checks when UI changes
  • Created/modified automated E2E tests in Cypress
    • Including cypress-axe checks when UI changes
  • Performed a11y testing:
    • Checked responsiveness in mobile, tablet, and desktop
    • Checked keyboard navigability
    • Tested with VoiceOver in Safari
    • Checked VO's rotor menu for landmarks, page heading structure and links
    • Used a browser a11y tool to check for issues (WAVE, axe, ANDI or Accessibility addon tab for Storybook)
  • Requested a design review for user-facing changes

As code reviewer(s), I have

  • Pulled this branch locally and tested it
  • Reviewed this code and left comments
  • Checked that all code is adequately covered by tests
  • Made it clear which comments need to be addressed before this work is merged
  • Considered marking this as accepted even if there are small changes needed
  • Performed a11y testing:
    • Checked responsiveness in mobile, tablet, and desktop
    • Checked keyboard navigability
    • Tested with VoiceOver in Safari
    • Checked VO's rotor menu for landmarks, page heading structure and links
    • Used a browser a11y tool to check for issues (WAVE, axe, ANDI or Accessibility addon tab for Storybook)

As a designer reviewer, I have

  • Checked in the design translated visually
  • Checked behavior
  • Checked different states (empty, one, some, error)
  • Checked for landmarks, page heading structure, and links
  • Tried to break the intended flow
  • Performed a11y testing:
    • Checked responsiveness in mobile, tablet, and desktop
    • Checked keyboard navigability
    • Tested with VoiceOver in Safari
    • Checked VO's rotor menu for landmarks, page heading structure and links
    • Used a browser a11y tool to check for issues (WAVE, axe, ANDI or Accessibility addon tab for Storybook)

Screenshots

image

@shortcut-integration
Copy link

This pull request has been linked to Shortcut Story #145: Remove beta UI elements.

@suzubara suzubara marked this pull request as ready for review March 16, 2022 22:15
source: '/training-and-education/force-multiplier-program',
destination: '/404',
permanent: false,
},
]
},
async rewrites() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Will we eventually reorg the file structure to get rid of the beta dir and these rewrites?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes! in sc-146

@jbecker01
Copy link
Contributor

jbecker01 commented Mar 23, 2022

As code reviewer(s), I have

  • Pulled this branch locally and tested it
  • Reviewed this code and left comments
  • Checked that all code is adequately covered by tests
  • Made it clear which comments need to be addressed before this work is merged
  • Considered marking this as accepted even if there are small changes needed
  • Performed a11y testing:
    • Checked responsiveness in mobile, tablet, and desktop
    • Checked keyboard navigability
    • Tested with VoiceOver in Safari
    • Checked VO's rotor menu for landmarks, page heading structure and links
    • Used a browser a11y tool to check for issues (WAVE, axe, ANDI or Accessibility addon tab for Storybook)

Tested across Chrome, FF, and Edge on Windows. Also tested on Mac Safar and Chrome.
Successfully ran through the test script here

Copy link
Contributor

@jbecker01 jbecker01 left a comment

Choose a reason for hiding this comment

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

👍

@suzubara suzubara merged commit d0f180f into main Mar 23, 2022
@suzubara suzubara deleted the sc-145/remove-beta-gate branch March 23, 2022 15:03
This was referenced Mar 23, 2022
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.

3 participants