header links navigates to unknown page

[sag1v]

After migrating from storybook v5 to v6 i have noticed a strange behavior regarding auto generated header links. After a quick search i found out why it is happening (but not sure how it worked until now with previous version).

Given this check in addons-links and the fact that all header anchors are actually an SVG elements, this check always evaluates to true (SVG element != HTML Element).

Because we return we are not triggering the preventDefault method of the event hence the link navigates to a different page (a none existing page).

Here is a screenshot of the gist of it

To Reproduce
I just created a fresh repo with npx sb@next repro and this bug shows up, i didn't change anything.

System
Environment Info:

System:
OS: macOS 11.6
Binaries:
Node: 12.16.3 - ~/.nvm/versions/node/v12.16.3/bin/node
Yarn: 1.22.5 - ~/.y/bin/yarn
npm: 6.14.4 - ~/.nvm/versions/node/v12.16.3/bin/npm
Browsers:
Chrome: 96.0.4664.55
Firefox: 85.0.2
Safari: 15.0

[cgreene-st]

I'm also seeing this in the latest version 6.4.0. Thanks for flagging the bug 👍

You can also see it live here by clicking the headings in the docs page - https://storybookjs.netlify.app/official-storybook/?path=/story/addons-docs-markdown-docs--page

[sag1v]

I'm not sure whats the correct fix for this (should we test against SVG elements as well?) nor what could be a valid workaround until this is fixed.

Maybe @shilman can advise?

[sag1v]

Hi @shilman Do you know if this is being worked on

[shilman]

Not yet. Please feel free to take a crack at it if it's blocking you!

[sag1v]

OK I think i found the issue and a way to fix it. I'm not sure how good is this fix though.

It turns out that in mdx.tsx we are emitting NAVIGATE_URL with only the hash as the url value instead of appending it to the entire location.search string.

Because we are passing options.plain: true (somewhere, not sure exactly where) then we just call react-router's navigate with the hash. see router.tsx.

My suggested solution is to bring back generateHrefWithHash (who got deleted a year ago) with some modifications:

function generateHrefWithHash(hash: string): string {
  const location = window.parent ? window.parent.location : window.location;
  const href = `${location.search}${hash}`;
  return href;
}

function navigate(url: string) {
  const urlWithHash = generateHrefWithHash(url);
  addons.getChannel().emit(NAVIGATE_URL, urlWithHash);
}

I'm not sure this is the best solution or naming but anyway it seems to work fine.

@shilman @ndelangen WDYT?

PR - #17080

[shilman]

Jiminy cricket!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.5.0-alpha.9 containing PR #17134 that references this issue. Upgrade today to the @next NPM tag to try it out!

npx sb upgrade --prerelease

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

[StanislavPonomarenko]

I've updated to prerelease.
I use tsx component for navigation via mdx file

<NavBanner
title="Colors"
desc="Primary, Secondary and Support colors"
links={[
{ path: '#identity-colors', text: 'Identity Colors' },
{ path: '#colors', text: 'Colors' },
]}

and in the same file I've used anchor link <a href="#identity-colors">Test</a>

In result links in component do not work but test anchor link works. https://www.screencast.com/t/JStshkoy
here is component code https://www.screencast.com/t/VEfhp3wy

[shilman]

w00t!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.4.10 containing PR #17134 that references this issue. Upgrade today to the @latest NPM tag to try it out!

npx sb upgrade

[StanislavPonomarenko]

Thank you for your comment but issue is still there after update to 6.4.10
here is a short video file https://www.screencast.com/t/PFo1KJ44covD

so anchor links in mdx file works, but if I use custom component with such links in mdx file those links won't work.
If you need some info from my side I would give it to you.

[shilman]

@StanislavPonomarenko we don't support links from custom components. please open a separate issue.