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

Angular: Fix support for non-roman alphabets in story titles #16931

Merged
merged 2 commits into from
Dec 7, 2021
Merged

Angular: Fix support for non-roman alphabets in story titles #16931

merged 2 commits into from
Dec 7, 2021

Conversation

nagashimam
Copy link

Issue: #15147

What I did

@storybook/angular uses story id as HTML tag. So using non-ascii characters in story title generates invalid HTML tag. I added a new method to create a valid HTML tag from story id. This method returns storyId when it doesn't contain any non-ASCII alphanumerics. Otherwise, it generates an HTML tag from storyId by removing non-ASCII alphanumerics from storyId, prefixing "sb-", and suffixing "-component"

How to test

  • Is this testable with Jest or Chromatic screenshots?
  • Does this need a new example in the kitchen sink apps?
  • Does this need an update to the documentation?

My answer is no to all of these questions. I added tests in app/angular/src/client/preview/angular-beta/RendererFactory.test.ts to test having non-ascii characters in story id doesn't break stories now.

@nx-cloud
Copy link

nx-cloud bot commented Dec 7, 2021

Nx Cloud Report

CI ran the following commands for commit cf4ec52. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

Status Command
#000000 nx run-many --target=prepare --all --parallel --max-parallel=15

Sent with 💌 from NxCloud.

Copy link
Contributor

@ThibaudAV ThibaudAV left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A small detail, it could happen that 2 stories end up with the same Id even if they didn't have the same name before. 🤔
e.g : 'my-スト' id === 'my-トス' === 'sb-my--component'

but maybe it's a marginal case 🤷‍♂️ so LGTM 🚀

@shilman shilman changed the title Fix #15147 Angular: Storybook 6.3 stopped supporting Cyrillic in title in stories Angular: Fix support for non-roman alphabets in story titles Dec 7, 2021
@shilman shilman merged commit a5dac26 into storybookjs:next Dec 7, 2021
@shilman shilman added patch:yes Bugfix & documentation PR that need to be picked to main branch patch:done Patch/release PRs already cherry-picked to main/release branch labels Dec 7, 2021
shilman added a commit that referenced this pull request Dec 9, 2021
Angular: Fix support for non-roman alphabets in story titles
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
angular bug patch:done Patch/release PRs already cherry-picked to main/release branch patch:yes Bugfix & documentation PR that need to be picked to main branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants