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.

Sign up for GitHub

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

Jump to bottom

[CL-141] Custom storybook theme #5569

Merged
merged 11 commits into from
Nov 16, 2023
Merged

[CL-141] Custom storybook theme #5569

merged 11 commits into from
Nov 16, 2023

Conversation

danielleflinn
Copy link
Member

@danielleflinn danielleflinn commented Jun 5, 2023 •
edited
Loading

Type of change

- [ ] Bug fix
- [ ] New feature development
- [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [X] Other

Objective

Add Bitwarden logo and other branding styles to the Storybook site.

Code changes

  • .storybook/manager.js added file to set custom themes

Before you submit

  • Please add unit tests where it makes sense to do so (encouraged but not required)
  • If this change requires a documentation update - notify the documentation team
  • If this change has particular deployment requirements - notify the DevOps team
  • Ensure that all UI additions follow WCAG AA requirements

@danielleflinn danielleflinn requested a review from Hinton June 5, 2023 22:54
@github-actions github-actions bot added the needs-qa Marks a PR as requiring QA approval label Jun 5, 2023
@danielleflinn
Copy link
Member Author

@Hinton it would be nice if we could override the favicon with this PR; but it looks like this isn't documented well. Any ideas how we might do this?

This seems like the most promising solution I've found.

@Hinton
Copy link
Member

Hinton commented Jun 6, 2023

@danielleflinn Any way to get light and dark themes?

@danielleflinn
Copy link
Member Author

@Hinton from my research there isn't an easy way to override both themes. But it sounds Storybook might work on a fix for it soon.

Hinton
Hinton reviewed Jun 13, 2023
View reviewed changes
.storybook/customTheme.js Outdated Show resolved Hide resolved
@danielleflinn
Added support for dark theme
993a184 
- moved  custom theme into manager.js
- add overrides for dark theme
- added logic for detecting dark/light theme
@danielleflinn danielleflinn removed the needs-qa Marks a PR as requiring QA approval label Aug 24, 2023
@danielleflinn danielleflinn marked this pull request as ready for review August 24, 2023 22:40
@danielleflinn danielleflinn requested review from a team as code owners August 24, 2023 22:41
@danielleflinn danielleflinn requested a review from Hinton August 24, 2023 22:41
@bitwarden-bot

This comment was marked as off-topic.

Sign in to view
Hinton
Hinton reviewed Sep 7, 2023
View reviewed changes
package.json Outdated Show resolved Hide resolved
@danielleflinn @Hinton
Update package.json
7bf6bfa 
Co-authored-by: Oscar Hinton <Hinton@users.noreply.github.com>
@danielleflinn danielleflinn changed the title Custom storybook theme [CL-141] Custom storybook theme Oct 27, 2023
@danielleflinn danielleflinn requested a review from a team as a code owner October 27, 2023 17:18
willmartian
willmartian reviewed Oct 27, 2023
View reviewed changes
Copy link
Contributor

@willmartian willmartian left a comment

Choose a reason for hiding this comment

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

Any idea why this font has a different vertical alignment compared to the Storybook default?

With OpenSans:

image

Storybook default:

image

.storybook/manager.js Outdated Show resolved Hide resolved
.storybook/manager.js Outdated Show resolved Hide resolved
danielleflinn and others added 2 commits October 27, 2023 12:39
@danielleflinn @willmartian
Update .storybook/manager.js
3c757fc 
updated logo link to link to link of deployment

Co-authored-by: Will Martin <contact@willmartian.com>
@danielleflinn @willmartian
Update .storybook/manager.js
6fdbad7 
Co-authored-by: Will Martin <contact@willmartian.com>
@danielleflinn
Copy link
Member Author

Any idea why this font has a different vertical alignment compared to the Storybook default?

@willmartian I'm not sure 🤔 Looking at the Chromatic preview in chrome, on my machine I don't see the mis-alignment shown in your screenshots. What browser are you using?
Screenshot 2023-10-27 at 12 54 12 PM

I'm open thought to reverting the type changes and just merging in the logo + color changes.

@willmartian
Copy link
Contributor

willmartian commented Oct 27, 2023 •
edited
Loading

I'm open thought to reverting the type changes and just merging in the logo + color changes.

@danielleflinn Yeah, let's just revert the Open Sans change for now. Everything else looks great :D

Hinton
Hinton requested changes Nov 16, 2023
View reviewed changes
.storybook/manager.js Outdated Show resolved Hide resolved
@danielleflinn @Hinton
Update .storybook/manager.js
18918be 
Co-authored-by: Oscar Hinton <Hinton@users.noreply.github.com>
@danielleflinn danielleflinn requested a review from Hinton November 16, 2023 14:23
@danielleflinn danielleflinn merged commit bad1f1b into master Nov 16, 2023
4 checks passed
@danielleflinn danielleflinn deleted the Custom-storybook-theme branch November 16, 2023 14:24
BlackDex pushed a commit to BlackDex/bitwarden-clients that referenced this pull request Nov 21, 2023
@danielleflinn @BlackDex
[CL-141] Custom storybook theme (bitwarden#5569)
80ce89c 
Add Bitwarden logo and other branding styles to the Storybook site.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Hinton Hinton Hinton approved these changes

@willmartian willmartian Awaiting requested review from willmartian

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@danielleflinn @Hinton @bitwarden-bot @willmartian