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: add launch darkly flag for dark mode toggle #787

Merged
merged 16 commits into from
Oct 4, 2022

Conversation

gidjin
Copy link
Contributor

@gidjin gidjin commented Sep 27, 2022

SC-774

Proposed changes

Configure LaunchDarkly Federal edition and setup ThemeToggle to use the first feature flag.

Reviewer notes

I setup 4 environments (localhost, dev, staging, and production). Localhost is what should be setup for developer boxes. See the readme changes for how to get it working locally.

I didn't add unit tests for the flag code in ThemeToggle because I figured we remove that code once the feature is fully released. But if you feel differently I'm happy to add a couple.

TODO add the environment variable for dev to the AWS dev environment.

Setup

Start everything normally

yarn cms:up
yarn dev

See that the ThemeToggle is show or hidden depending on the setting in the localhost environment in launch darkly dashboard. If you toggle it there you will see the button hide and show automatically!


Code review steps

As the original developer, I have

  • Met the acceptance criteria, or will meet them in subsequent PRs or stories
  • Created new stories in Storybook if applicable
    • Checked that all Storybook accessibility checks are passing
  • Created/modified automated E2E tests in Cypress
    • Including cypress-axe checks when UI changes
    • Checked that the E2E test build is not failing

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
    • Checked that the E2E test build is not failing
  • 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)

As a test user, I have

  • Run through the Test Script:
    • On commercial internet in IE11
    • On commercial internet in Edge
    • On commercial internet in Firefox
    • On commercial internet in Chrome
    • On commercial internet in Safari
    • On NIPR in IE11
    • On NIPR in Firefox
    • On NIPR in Chrome
    • On NIPR in Safari
    • On a mobile device in Firefox
    • On a mobile device in Chrome
    • On a mobile device in Safari
  • Added any anomalous behavior to this PR

@shortcut-integration
Copy link

This pull request has been linked to Shortcut Story #774: Implement Launch Darkly flags for Dark Mode.

@gidjin gidjin requested a review from jcbcapps September 27, 2022 16:59
@gidjin gidjin marked this pull request as ready for review September 27, 2022 16:59
@jbecker01
Copy link
Contributor

I noticed that the dark mode setting isn't persistent across all the pages of the portal. Is that something that would be covered under #778 ?

Copy link
Contributor

@jcbcapps jcbcapps left a comment

Choose a reason for hiding this comment

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

Awesome!

@jbecker01
Copy link
Contributor

Looks good!

streamUrl: 'https://clientstream.launchdarkly.us',
eventsUrl: 'https://events.launchdarkly.us',
},
})(USSFPortalApp as unknown as ComponentType)
Copy link
Contributor

Choose a reason for hiding this comment

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

i love typescript 😭 😂

Copy link
Contributor

@abbyoung abbyoung left a comment

Choose a reason for hiding this comment

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

works great, very exciting!

@gidjin gidjin merged commit 397dccd into main Oct 4, 2022
@gidjin gidjin deleted the sc-774/add-launch-darkly-flag-for-dark-mode-toggle branch October 4, 2022 23:31
@gidjin gidjin mentioned this pull request Oct 5, 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.

4 participants