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

fix: storybook dark theme #1065

Merged
merged 17 commits into from
Jul 19, 2023
Merged

fix: storybook dark theme #1065

merged 17 commits into from
Jul 19, 2023

Conversation

gidjin
Copy link
Contributor

@gidjin gidjin commented Jul 12, 2023

SC-2131

Proposed changes

The plugin we used for toggling the theme in storybook no longer works. This PR removes it and adds a custom storybook theme button to toggle the theme. This is based on storybook example for decorator

Reviewer notes

This doesn't address configuring happo to switch the themes, but that's in a story I'm picking up next. Also there are some deprecated way of writing stories that this PR fixed, see Pagination and GovBanner files for an example.

Setup

Start the system

yarn services:up
yarn dev
cd ../ussf-portal-cms
yarn dev

Login to the portal http://localhost:3000

Start storybook

yarn storybook

Login to storybook http://localhost:6006, though the command above should open it for you


Code review steps

As the original developer, I have

  • Met the acceptance criteria
  • Created new stories in Storybook if applicable

As a reviewer, I have

Check out our How to review a pull request document.


Screenshots

SCR-20230717-hs1
SCR-20230717-hsg
SCR-20230717-hss

@shortcut-integration
Copy link

This pull request has been linked to Shortcut Story #2131: Support theme display in storybook.

@gidjin gidjin force-pushed the sc-2131/fix-storybook-dark-theme branch from 0916467 to 70a3129 Compare July 12, 2023 20:55
@gidjin gidjin force-pushed the sc-2131/fix-storybook-dark-theme branch from 70a3129 to 2992df4 Compare July 12, 2023 22:00
@gidjin gidjin force-pushed the sc-2131/fix-storybook-dark-theme branch from 84e0c1c to 0a9abdf Compare July 14, 2023 22:35
@gidjin gidjin marked this pull request as ready for review July 17, 2023 19:49
@gidjin gidjin requested review from a team as code owners July 17, 2023 19:49
@shkeating
Copy link
Contributor

looks good! my only comments are small and not necessarily worth addressing

  • it would be nice if we could just click on the button that says "light" or "dark" and not have to go through the dropdown. - if you switch color themes you have the refresh the page to re run the accessibility checker or it throws an error. this also happens if you switch color themes while the accessibility tester is running

@gidjin
Copy link
Contributor Author

gidjin commented Jul 19, 2023

looks good! my only comments are small and not necessarily worth addressing

  • it would be nice if we could just click on the button that says "light" or "dark" and not have to go through the dropdown. - if you switch color themes you have the refresh the page to re run the accessibility checker or it throws an error. this also happens if you switch color themes while the accessibility tester is running

I did want a button also, but the pull down was in the docs and I didn't have a chance to dig into it further to see about making it a button. That said I think I have to modify things a bit the story to have happo see both themes so I'll revisit the button there. I'll also see what I can do for the accessibility tester there too.

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.

LGTM!

@gidjin gidjin merged commit c5deda3 into main Jul 19, 2023
@gidjin gidjin deleted the sc-2131/fix-storybook-dark-theme branch July 19, 2023 21:33
gidjin pushed a commit that referenced this pull request Sep 11, 2023
## [4.23.0](4.22.1...4.23.0) (2023-09-11)


### Features

* Add resolvers and mutations for managing weather widget ([#1075](#1075)) ([a652766](a652766))
* Add support for CTA to link a document ([#1082](#1082)) ([fa0a6bb](fa0a6bb))
* Change h3 from book weight to bold weight ([#1076](#1076)) ([c92ae65](c92ae65))
* Create weather widget ([#1079](#1079)) ([3e6b91d](3e6b91d))
* Load user personnel data into portal ([#1088](#1088)) ([fe0a1d8](fe0a1d8))
* new loader animation ([#1085](#1085)) ([b099904](b099904))


### Bug Fixes

* **deps:** update dependencies ([#1059](#1059)) ([441a35c](441a35c))
* design: dark mode adjustments ([#1091](#1091)) ([9c67a4a](9c67a4a))
* design: Footer styling refactor for accessibility and responsive design improvements ([#1081](#1081)) ([18dd17f](18dd17f))
* local personnel-api service targets builder stage only ([#1084](#1084)) ([94950eb](94950eb))
* storybook dark theme ([#1065](#1065)) ([c5deda3](c5deda3))
* storybook deploy ([#1072](#1072)) ([ee21044](ee21044))


### Security Improvements

* **deps:** update auto-instrumentations-web and auto-instrumentations-node ([#1087](#1087)) ([71236b8](71236b8))
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