storybook: Theme variable brandTitle incorrectly applied

Describe the bug When we use the theme variable brandTitle on its own, it should replace the Storybook logo in the upper left with a string. Instead, it simply modifies the alt tag for the existing image.

Repro in official-storybook:

 theme: create({ brandTitle: 'foo', colorPrimary: 'hotpink', colorSecondary: 'orangered' }),

System:

  • Version: 5.0.0-rc.11

About this issue

  • Original URL
  • State: closed
  • Created 5 years ago
  • Reactions: 5
  • Comments: 22 (14 by maintainers)

Most upvoted comments

Hey @ndelangen, I actually found a workaround for our use case. It is a bit hacky but it works nicely and does exactly what we want it to do.

brandTitle: `<img src="./build/assets/logo/logo.svg" width="150px"/> YOBI Design\n\n ${appVersion}`

I am still happy to discuss on Discord towards the end of the week - I should have time this Friday so will contact you there. I have signed up for the group already.

Thanks again for all your time and help.

@shilman what if that text is dynamic i.e. a version number. I cannot include it as part of the brandImage (since I would have to regenerate the asset everytime) but I still think it would be valuable information to add as a subtitle for instance. Could you point me to where the brandImage code is ? I can help implement a subtitle if needed 😃 Happy to discuss our use case further but I think there is some value in either having both (image + title) or having a subtitle

Hey @ndelangen, I actually found a workaround for our use case. It is a bit hacky but it works nicely and does exactly what we want it to do.

brandTitle: `<img src="./build/assets/logo/logo.svg" width="150px"/> YOBI Design\n\n ${appVersion}`

I am still happy to discuss on Discord towards the end of the week - I should have time this Friday so will contact you there. I have signed up for the group already.

Thanks again for all your time and help.

Thanks @miguelyoobic95 for this workaround. Indeed I ran into the same issue where I want to show a dynamic version number. I feel like Storybook shouldn’t be opinionated about this. If I want to show a brandTitle AND brandImage, I should be allowed to do it.

@shilman I am specifying a custom brandImage but I would like to add some text underneath, in this case the version number of the app. Is there anywhere to separate these two ? seems like a valid use case for me - a subtitle could be an alternative possibility maybe

Jeepers creepers!! I just released https://github.com/storybooks/storybook/releases/tag/v5.1.0-alpha.7 containing PR #6120 that references this issue. Upgrade today to try it out!

Because it’s a pre-release you can find it on the @next NPM tag.

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

what people are missing is they need to clear the image and set the text

Just ran into this issue myself. I think the main problem is that the default Storybook logo/svg contains text so as a dev I assumed the brandTitle would replace the text Storybook and leave the image, the little pink book. To me the book seems like the brandImage and the text would be the brandTitle.

It’s not clear from documentation or intuition that you can only have one OR the other, image OR text. I would recommend one of the following:

  1. Documenting better
  2. change the implementation to have a logo/image AND a text title with the book as the default brandImage and “Storybook” as the default brandTitle.

IMO the second option is more intuitive and easier to understand without having to dig/search through the Docs and would allow any devs to easily add their library’s logo AND title without having to make an image/svg including the logo and text.

The source of this issue is all these checks for null vs undefined in this component:

https://github.com/storybooks/storybook/blob/b19ae6b9e44e8759a4735169059cfa05f104d5e9/lib/ui/src/components/sidebar/SidebarHeading.js#L68-L93

Looking at the relevant stories, it looks like you are supposed to set the brandImage to null if you want no image:

https://github.com/storybooks/storybook/blob/b19ae6b9e44e8759a4735169059cfa05f104d5e9/lib/ui/src/components/sidebar/SidebarHeading.stories.js#L66-L79

@ndelangen / @domyen I’m not sure why the decision was to use null here rather than false, but in either case this is working as designed, what people are missing is they need to clear the image and set the text:

 theme: create({ brandTitle: 'foo', brandImage: null, colorPrimary: 'hotpink', colorSecondary: 'orangered' }),

A documentation issue?