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

Selected sidebar item doesn't respect theme colors #6335

Closed
vpicone opened this issue Mar 28, 2019 · 4 comments
Closed

Selected sidebar item doesn't respect theme colors #6335

vpicone opened this issue Mar 28, 2019 · 4 comments

Comments

@vpicone
Copy link
Contributor

vpicone commented Mar 28, 2019

Describe the bug
Setting theme.color.lightest and theme.color.inverseText doesn't impact the color of selected items in the sidebar (they remain #FFFFFF regardless)

From the sidebar component, it looks like theme.color.lightest should effectively color the component but it it looks like it is overridden by div#accordion--skeleton for some reason.

https://github.com/storybooks/storybook/blob/a28c59592ee5d43152b684958ee83b8fc55adbcd/lib/ui/src/components/sidebar/SidebarItem.js#L73

To Reproduce
Steps to reproduce the behavior:

  1. Update all of the values for colors in a theme to black.
  2. The selected text remains #FFFFFF no matter what

Expected behavior
The selected item should have text according to the theme value that is used in the sidebar item component when selected (theme.color.lightest)

Screenshots
If applicable, add screenshots to help explain your problem.
Screen Shot 2019-03-28 at 10 44 03 AM

Code snippets

addParameters({
  options: {
    theme: {
      ...theme,
      color: { ...theme.color, lightest: 'red', inverseText: 'red' },
    },
  },
});

System:

  • Browser: Firefox
  • Framework: react
  • Version: [e.g. 5.0.5]

Additional context
Trying to use a lighter secondary color for the selected items but finding it impossible to change the color of the selected items.

@TimoGlastra
Copy link

I looked into this issue a bit. As you see in the ThemeVars interface below, theme.color.lightest is not a property you can set.

https://github.com/storybooks/storybook/blob/ed9bcff0d88da47c4f014ea78eb8fcb6458e5821/lib/theming/src/base.ts#L97-L131

The color.lightest is always #FFFFFF because the base theme definition is used for this attribute. I think there are two possible solutions here:

  1. Infer the color.lightest attribute from an already existing ThemeVars attribute.
  2. Create a new attribute in ThemeVars to set the the color.lightest (and maybe other) attributes.

I don't think option 1 is the best solution as it will change the look of the site quite a bit by inferring color.lightest from an already existing ThemeVars attribute.

Is there a particular reason why the underlying theme properties aren't customisable? Or am i totally wrong here by saying that it is not customisable? I get that it is convenient for creating themes that have a consistent style, but the option would be nice.

Since this is my first time fixing an issue for storybook i would like some input before moving forward.

@vpicone
Copy link
Contributor Author

vpicone commented Mar 31, 2019

It seems like it should use the inverseText theme property or a new property all together.

@shilman shilman modified the milestones: 5.0.x, 5.1.x Jun 5, 2019
@ndelangen
Copy link
Member

ndelangen commented Jun 17, 2019

When #6806 drops we'll support full custom theme options, including this property.

@stale
Copy link

stale bot commented Jul 8, 2019

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants