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

[Storybook] Space insertion in titles with diacritics characters #12534

Closed
cesarsl opened this issue Sep 21, 2020 · 8 comments
Closed

[Storybook] Space insertion in titles with diacritics characters #12534

cesarsl opened this issue Sep 21, 2020 · 8 comments

Comments

@cesarsl
Copy link

cesarsl commented Sep 21, 2020

Describe the bug
When using the characters with diacritics (' " ` ´ ^ ¸ ~), Storybook inserts white space characters between them and the ones before and after.

To Reproduce
Steps to reproduce the behavior:

  1. Create a story and define its <Meta title="" /> with a word and some diacritic character attached to it. Example: São
  2. Start Storybook
  3. Go to the story
  4. Hover the mouse over the browser title or verifiy the <title> tag inside the page's html

Expected behavior
Would expect no white spaces between those characters

Screenshots
Screenshot from 2020-09-21 10-26-36
Screenshot from 2020-09-21 10-41-09

Code snippets

import { Meta } from '@storybook/addon-docs/blocks';

<Meta title="Introdução/Sobre o Foundation" />

# Sobre o Foundation

System:

Environment Info:

  System:
    OS: Linux 4.19 Debian GNU/Linux 10 (buster) 10 (buster)
    CPU: (4) x64 Intel(R) Core(TM) i5-7500 CPU @ 3.40GHz
  Binaries:
    Node: 12.18.1 - ~/.nvm/versions/node/v12.18.1/bin/node
    Yarn: 1.22.5 - ~/.yarn/bin/yarn
    npm: 6.14.5 - ~/.nvm/versions/node/v12.18.1/bin/npm
  Browsers:
    Chrome: 85.0.4183.102
    Firefox: 68.12.0esr
  npmPackages:
    @storybook/addon-a11y: ^6.0.21 => 6.0.21 
    @storybook/addon-actions: ^6.0.21 => 6.0.21 
    @storybook/addon-controls: ^6.0.21 => 6.0.21 
    @storybook/addon-docs: ^6.0.21 => 6.0.21 
    @storybook/addon-essentials: ^6.0.21 => 6.0.21 
    @storybook/addon-knobs: ^6.0.21 => 6.0.21 
    @storybook/addon-links: ^6.0.21 => 6.0.21 
    @storybook/theming: ^6.0.21 => 6.0.21 
    @storybook/web-components: ^6.0.21 => 6.0.21

Additional context
Reproduced in Linux, macOS and Windows

@shilman
Copy link
Member

shilman commented Sep 22, 2020

Any chance you can contribute a fix @cesarsl?

@cesarsl
Copy link
Author

cesarsl commented Sep 22, 2020

I will give a try as soon as I can. Thanks for taking the time to look at this issue.

@cesarsl
Copy link
Author

cesarsl commented Sep 22, 2020

Sorry, but I can't even run storybook in developer mode.

Followed all instructions in Contributing to Storybook, but I keep receiving this error:

Error: ENOSPC: System limit for number of file watchers reached

Already tried to adjust my fs.inotify.max_user_watches=524288, as some people pointed out around the web, but no success either.

@shilman
Copy link
Member

shilman commented Sep 25, 2020

Try running:

yarn build core client-api web-components [other packages you're developing] --watch

@cesarsl
Copy link
Author

cesarsl commented Sep 25, 2020

Great, found the piece of code in charge of building the titles.

const nonAlphanumSpace = /[a-z0-9 ]/gi;
const doubleSpace = /\s\s/gi;
const replacer = (match: string) => ` ${match} `;

const addExtraWhiteSpace = (input: string) =>
  input.replace(nonAlphanumSpace, replacer).replace(doubleSpace, ' ');

const getDescription = (item: Item) => {
  if (isRoot(item)) {
    return item.name ? `${item.name} ⋅ Storybook` : 'Storybook';
  }
  if (isGroup(item)) {
    return item.name ? `${item.name} ⋅ Storybook` : 'Storybook';
  }
  if (isStory(item)) {
    const { kind, name } = item;
    return kind && name ? addExtraWhiteSpace(`${kind} - ${name} ⋅ Storybook`) : 'Storybook';
  }

  return 'Storybook';
};

The nonAlphanumSpace const has a regular expression that only accounts for /[^a-z0-9\s]/gi (the \s in the regex is just a whitespace, changed for better readablity). All characters with diacritics and letters from other langues would be considered title separators and the function will add the extra space between those characters.

Talking about separators: I understand that the menu building function use / as a separator, but when comes to the page title, if you uses any nonAlphanumSpace, all others symbols can be used as title separators.

We could just place a better regex, but I don't think we really need.

All that logic can be simplified to:

const splitTitleAddExtraSpace = (input: string) => input.split('/').join(' / ');

const getDescription = (item: Item) => {
  if (isRoot(item)) {
    return item.name ? `${item.name} ⋅ Storybook` : 'Storybook';
  }
  if (isGroup(item)) {
    return item.name ? `${item.name} ⋅ Storybook` : 'Storybook';
  }
  if (isStory(item)) {
    const { kind, name } = item;
    return kind && name ? splitTitleAddExtraSpace(`${kind} - ${name} ⋅ Storybook`) : 'Storybook';
  }

  return 'Storybook';
};

This is just a suggestion, because I don't know why these decisions were made.

If someone would care to explain, I'd appreciated that.

Cheers.

@cesarsl
Copy link
Author

cesarsl commented Sep 28, 2020

@shilman Do we have to close this issue, or will it close itself?

@shilman
Copy link
Member

shilman commented Sep 28, 2020

Will close itself once i release your fix @cesarsl

@shilman
Copy link
Member

shilman commented Sep 30, 2020

Egads!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.1.0-alpha.18 containing PR #12583 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 Sep 30, 2020
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

2 participants