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

Toolbars: Should show name if no icon is configured #11474

Closed
BPScott opened this issue Jul 9, 2020 · 3 comments
Closed

Toolbars: Should show name if no icon is configured #11474

BPScott opened this issue Jul 9, 2020 · 3 comments
Assignees
Milestone

Comments

@BPScott
Copy link

BPScott commented Jul 9, 2020

Is your feature request related to a problem? Please describe.

I'm not a fan of mystery-meat navigation. I like to see what an item in a toolbar does without needing to hover over it to see a tooltip. That's why when configuring addon-controls I specify a title but no icon and it shows the title text ("New Design Language") in the toolbar.

When trialing Storybook v6 I tried porting to use addon-toolbar and args however addon-toolbar does not have this fallback behaviour if an icon is absent - it displays an empty box.

Expected behaviour

When specifying a toolbar without a icon key then the toolbar item should use the name of the type in the toolbar, in a similar way to addon-controls.

Sample of toolbar config with omitted icon:

export const globalTypes = {
  designLanguage: {
    name: 'New Design Language',
    description: 'New DL',
    defaultValue: 'off',
    // when this is ommitted the name should be used instead
    // icon: 'circlehollow',
    toolbar: {
      items: [
        {title: 'Disabled', value: 'off'},
        {title: 'Enabled - Light Mode', value: 'light'},
        {title: 'Enabled - Dark Mode', value: 'dark'},
      ],
    },
  },
};

I think the fix for this requires updating the following code to include the name in the children on IconButton, if the icon and selectedItem.icon are absent:

<IconButton key={name} active={active} title={description}>
<Icons icon={(selectedItem && selectedItem.icon) || icon} />
</IconButton>

@shilman
Copy link
Member

shilman commented Jul 9, 2020

Yo-ho-ho!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.0.0-rc.1 containing PR #11475 that references this issue. Upgrade today to try it out!

You can find this prerelease on the @next NPM tag.

Closing this issue. Please re-open if you think there's still more to do.

@shilman shilman closed this as completed Jul 9, 2020
@alinalobast
Copy link

alinalobast commented Aug 18, 2021

Hi @shilman! Looks like same exact issue reappeared in v6.2.8. It used to work for us before for the following config, but suddenly stopped after we bumped to v.6.2.8 (or one of the earlier patch ones). Adding 'icon' parameter to the config below solves the issue.

export const globalTypes = {
    theme: {
        name: 'Theme',
        description: 'Global Theme',
        defaultValue: ThemeType.default,
        toolbar: {
            items: [ThemeType.default, ThemeType.dark, ThemeType.contrast],
        },
    },
    direction: {
        name: 'Direction',
        description: 'Direction value',
        defaultValue: "ltr",
        toolbar: {
            items: [{
                    value: "ltr",
                    title: "Left-to-right"
                },
                {
                    value: "rtl",
                    title: "Right-to-left"
                }
            ],
        }
    }
};

@shilman
Copy link
Member

shilman commented Aug 18, 2021

@IgorSzyporyn is this something you might be able to take a quick look at? ☝️ 🙏

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

3 participants