Skip to content

Conversation

@shobhitchittora
Copy link

@shobhitchittora shobhitchittora commented Jan 13, 2018

Closes: #2736
Fixes eslint errors with template.


This change is Reviewable

Copy link
Member

@Hypnosphi Hypnosphi left a comment

Choose a reason for hiding this comment

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

Cool, thanks. But this is just a snapshot for tests, actual templates live here: https://github.com/storybooks/storybook/tree/master/lib/cli/generators

Please check other generators as well, some of them may have the same story

@Hypnosphi
Copy link
Member

Maybe you could just add a11y/accessible-emoji to our root .eslintrc.js, and fix all the errors that would apper?

Here's contributing guide for developer setup: https://github.com/storybooks/storybook/blob/master/CONTRIBUTING.md

@shobhitchittora
Copy link
Author

shobhitchittora commented Jan 13, 2018

@Hypnosphi Thanks for pointing that out. I'll try to add the rule in .eslintrc.js.

@shobhitchittora
Copy link
Author

@Hypnosphi I tried adding the rule to the root eslint.js of the generators folder and found the following violations -

/Users/gaurav/Desktop/stuff/storybook/lib/cli/generators/METEOR/template/stories/index.stories.js
  13:33  error  Emojis should be wrapped in <span>, have role="img", and have an accessible description with aria-label or aria-labelledby  jsx-a11y/accessible-emoji

/Users/gaurav/Desktop/stuff/storybook/lib/cli/generators/REACT_NATIVE_SCRIPTS/template/storybook/stories/index.js
  23:7  error  Emojis should be wrapped in <span>, have role="img", and have an accessible description with aria-label or aria-labelledby  jsx-a11y/accessible-emoji

/Users/gaurav/Desktop/stuff/storybook/lib/cli/generators/REACT_NATIVE/template/storybook/stories/index.js
  23:7  error  Emojis should be wrapped in <span>, have role="img", and have an accessible description with aria-label or aria-labelledby  jsx-a11y/accessible-emoji

/Users/gaurav/Desktop/stuff/storybook/lib/cli/generators/REACT_SCRIPTS/template/src/stories/index.js
  13:33  error  Emojis should be wrapped in <span>, have role="img", and have an accessible description with aria-label or aria-labelledby  jsx-a11y/accessible-emoji

/Users/gaurav/Desktop/stuff/storybook/lib/cli/generators/REACT/template/stories/index.stories.js
  13:33  error  Emojis should be wrapped in <span>, have role="img", and have an accessible description with aria-label or aria-labelledby  jsx-a11y/accessible-emoji

/Users/gaurav/Desktop/stuff/storybook/lib/cli/generators/WEBPACK_REACT/template/stories/index.stories.js
  13:33  error  Emojis should be wrapped in <span>, have role="img", and have an accessible description with aria-label or aria-labelledby  jsx-a11y/accessible-emoji

Fixing them in this PR.

@Hypnosphi
Copy link
Member

Hypnosphi commented Jan 13, 2018

root eslint.js of the generators folder

Actually you can just add it in project root, because there might be violations in examples as well

@ndelangen
Copy link
Member

Snapshots don't match. Inspect your code changes or run 'yarn test --cli --update' to update them

@Hypnosphi
Copy link
Member

Hypnosphi commented Jan 13, 2018

@ndelangen that's because currently the only change is in snapshot itself, see comments above and "changes" tab

@Hypnosphi Hypnosphi added compatibility with other tools accessibility patch:yes Bugfix & documentation PR that need to be picked to main branch labels Jan 18, 2018
@Hypnosphi Hypnosphi self-assigned this Feb 17, 2018
@Hypnosphi
Copy link
Member

Superseded by #3011

@Hypnosphi Hypnosphi closed this Feb 18, 2018
@Hypnosphi Hypnosphi added patch:done Patch/release PRs already cherry-picked to main/release branch and removed patch:done Patch/release PRs already cherry-picked to main/release branch labels Mar 6, 2018
@yannbf yannbf removed the patch:yes Bugfix & documentation PR that need to be picked to main branch label Oct 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

accessibility compatibility with other tools patch:done Patch/release PRs already cherry-picked to main/release branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants