Skip to content

UI: Fix custom theme hover-color inconsistency#22262

Merged
ndelangen merged 5 commits into
storybookjs:nextfrom
yoshi2no:fix/hover-color-inconsistency
Sep 19, 2023
Merged

UI: Fix custom theme hover-color inconsistency#22262
ndelangen merged 5 commits into
storybookjs:nextfrom
yoshi2no:fix/hover-color-inconsistency

Conversation

@yoshi2no
Copy link
Copy Markdown
Contributor

@yoshi2no yoshi2no commented Apr 26, 2023

Issue: #22199

What I did

  • Currently, a tint of the default color.secondary in base.ts (code/lib/theming/src/base.ts) is used as background color in the bar, so changed to use theme.color.secondary as background color when BranchNode is hovered or focused.
  • The same applies to LeafNode

※ The reason why 0.93 is passed for the first argument of the transparentize function is because the value of background.hoverable in base.ts (code/lib/theming/src/base.ts) did so.

BranchNode(before) BranchNode(after) LeafNode(before) LeafNode(after)
スクリーンショット 2023-04-26 23 18 57 スクリーンショット 2023-04-26 23 24 29 スクリーンショット 2023-04-26 23 18 09 スクリーンショット 2023-04-26 23 25 42

How to test

  • generate a new theme using the create() function from storybook/theming as folows.
// code/ui/.storybook/manager.ts
import { addons } from '@storybook/manager-api';
import startCase from 'lodash/startCase.js';
import { create } from '@storybook/theming/create';

addons.setConfig({
  sidebar: {
    renderLabel: ({ name, type }) => (type === 'story' ? name : startCase(name)),
  },
  theme: create({
    base: 'light',
    colorSecondary: '#FF8688',
  }),
});
  • cd code
  • yarn storybook:ui
  • Check that the secondary-based color is used as background color when the BranchNode/LeafNode is hovered or focused.

Checklist

  • Make sure your changes are tested (stories and/or unit, integration, or end-to-end tests)
  • Make sure to add/update documentation regarding your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Maintainers

  • If this PR should be tested against many or all sandboxes,
    make sure to add the ci:merged or ci:daily GH label to it.
  • Make sure this PR contains one of the labels below.

["cleanup", "BREAKING CHANGE", "feature request", "bug", "documentation", "maintenance", "dependencies", "other"]

Copy link
Copy Markdown
Contributor

@cdedreuille cdedreuille left a comment

Choose a reason for hiding this comment

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

Thanks a lot for this change. I think this change makes a lot of sense.

@cdedreuille cdedreuille added maintenance User-facing maintenance tasks ci:normal labels Sep 19, 2023
@cdedreuille cdedreuille changed the title Fix: Custom theme hover-color inconsistency UI: Custom theme hover-color inconsistency Sep 19, 2023
@ndelangen ndelangen merged commit b2c69ee into storybookjs:next Sep 19, 2023
@vanessayuenn vanessayuenn changed the title UI: Custom theme hover-color inconsistency UI: Fix custom theme hover-color inconsistency Sep 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:normal maintenance User-facing maintenance tasks

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants