Storybook: Link to brandImage not working

Created on 14 Jan 2020  路  20Comments  路  Source: storybookjs/storybook

Describe the bug
During upgrade from 5.2.8 to 5.3.3 my brandImage stopped working.

I have a custom theme, using a static assets folder for the image

./storybook
 - ./public
   - logo.svg

Previously I had me theme defined like this:

import { create } from '@storybook/theming';
import logo from './public/logo.svg';

export default create({
  base: 'light',

  brandImage: logo,
  brandTitle: 'Custom - Storybook'
});

After updating to 5.3.3 I've moved my theming to manager.js, like so

import { addons } from '@storybook/addons';
import { create } from '@storybook/theming/create';
import logo from './public/logo.svg';

const theme = create({
  base: 'light',

  brandImage: `/${logo}`,
  brandTitle: 'Custom - Storybook'
});

addons.setConfig({
  panelPosition: 'bottom',
  theme
});

But the logo.svg does not show up when I start storybook using start-storybook -p 6006 -s ./.storybook/public.

If I however do a static build via build-storybook -s ./.storybook/public, the logo shows up correctly.

Webserver fetches the logo from /media/static/logo.svg in both cases. But it seems the local webserver started when starting storybook locally does not correctly allow fetching images from this folder.

System:
Environment Info:

System:
OS: macOS 10.15.2
CPU: (12) x64 Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
Binaries:
Node: 13.6.0 - ~/.nvm/versions/node/v13.6.0/bin/node
Yarn: 1.19.1 - /usr/local/bin/yarn
npm: 6.13.4 - ~/.nvm/versions/node/v13.6.0/bin/npm
Browsers:
Chrome: 79.0.3945.117
Safari: 13.0.4
npmPackages:
@storybook/addon-a11y: ^5.3.3 => 5.3.3
@storybook/addon-actions: ^5.3.3 => 5.3.3
@storybook/addon-docs: ^5.3.3 => 5.3.3
@storybook/addon-knobs: ^5.3.3 => 5.3.3
@storybook/addon-links: ^5.3.3 => 5.3.3
@storybook/addon-notes: ^5.3.3 => 5.3.3
@storybook/addon-viewport: ^5.3.3 => 5.3.3
@storybook/addons: ^5.3.3 => 5.3.3
@storybook/angular: ^5.3.3 => 5.3.3

bug has workaround high priority theming ui

Most helpful comment

@shilman would it be possible to backport the fix to the 5.3.x branch and republish? It's in the 5.3 hotlist, but it seems it was mistakenly merged to a prerelease branch. I'm still unable to update storybook because of this bug.

All 20 comments

A temporary solution until it's fixed:

import logoUrl from './public/logo.svg';

const theme = create({
  brandImage: (process.env.NODE_ENV === 'production') ? logoUrl : '/logo.svg',
build-storybook
start-storybook -p 6006 -s .storybook/public

Experiencing this issue too.

cc @ndelangen i think this is about moving from preview => manager

Added to my todo-list

My findings so far:

First I though maybe there's a loader missing for the manager
not it, the loader is there

Then I thought the file-loader might not be aware of the real publicPath or something
not it, it's working as intended, no further config would help I think

Now I'm thinking that in the express app, the routers for preview are preferred over the routes of the manager. The preview returns a 404, even though the manager has a route for the image. There are 2 webpack's being served, and being combined in express.
investigating this option

I have a PR open with a fix

If you're able I'd love a review @SuneRadich

I'm finding that the brand image URL is being defined, but locally it can't find static/media/brandImage.filename.png but when built the image exists in that location.

@jcousins-ynap yes, my PR fixes that

Nicely done @ndelangen much appreciated.

Still waiting for this? He doesn't seem to be very active.

@daphen he's on it. please chill.

@shilman Sorry, I didn't mean to come off as aggrevated. 馃槄 If he's on it then it's just a matter of waiting. I was under the impression that he wasn't here.

Jeepers creepers!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.0.0-alpha.7 containing PR #9646 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 would it be possible to backport the fix to the 5.3.x branch and republish? It's in the 5.3 hotlist, but it seems it was mistakenly merged to a prerelease branch. I'm still unable to update storybook because of this bug.

@cone56 it might be possible, but the change to make this work isn't trivial.. I agree it would likely be possible and is desired, but I'd have to find the time to backport it and test.

@cone56 Not a mistake: we typically merge everything into next, release on an alpha, and then patch bugfixes back to master when they are both high priority and low risk.

I'd say this one is fairly high priority but medium risk. If you'd like to help get the fix backported, can you test it out on the 6.0-alpha release and confirm that it's working well for you? Then @ndelangen and I can figure out whether we're comfortable doing the patch. Thanks!

@shilman @ndelangen I've just tested the latest alpha release and can confirm the fix is working well. If there's anything else I can help test, please let me know.

Thanks also for explaining your release process; I completely understand that these things are not as easy as they seem on the surface and can take time.

Thanks for testing @cone56! I'll see what we can do this week.

Anyway to get this fixed pushed to 5.3? The workaround requires making a static build :(

Whoopee!! I just released https://github.com/storybookjs/storybook/releases/tag/v5.3.18 containing PR #9646 that references this issue. Upgrade today to try it out!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

bpeab picture bpeab  路  70Comments

aericson picture aericson  路  97Comments

p3k picture p3k  路  61Comments

ilias-t picture ilias-t  路  73Comments

moimikey picture moimikey  路  67Comments