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

addon-a11y not working on latest next #4889

Closed
waldemarfm opened this issue Nov 29, 2018 · 11 comments
Closed

addon-a11y not working on latest next #4889

waldemarfm opened this issue Nov 29, 2018 · 11 comments

Comments

@waldemarfm
Copy link

waldemarfm commented Nov 29, 2018

Describe the bug
@storybook/addon-a11y not reporting a11y test results on next branch . It shows 0 passes and 0 violations on any story regardless of decorator being set or not.

To Reproduce

  1. Checkout next branch
  2. Start the official-storybook kitchen sink app or install/register add-on to any other app.
  3. Open any of the a11y stories with decorator set.
  4. Go to Accessibility panel.
  5. See 0 results on both passes/violations.

Expected behavior
Should display amount of test passes or violations respectively in each tab.

Screenshots
screen shot 2018-11-29 at 10 19 17 am

Code
It's related to the runA11yCheck function on the addon's declaration and the DOM being selected to run the aXe tests on:

const runA11yCheck = () => {
  const channel = addons.getChannel();
  const infoWrapper = document.getElementById('story-root').children;
  const wrapper = document.getElementById('root');

  axe.reset();
  axe.configure(axeOptions);
  axe
    .run(infoWrapper || wrapper)
    .then(results => channel.emit(CHECK_EVENT_ID, results), logger.error);
};

System:

  • Browser: any
  • Framework: all
  • Addons: a11y
  • Version: 4.0.9

Additional context
Tried this on some of the other kitchen sink apps with same result.

@waldemarfm waldemarfm changed the title addon-a11y no longer running on latest addon-a11y not working on latest next Nov 29, 2018
@hipstersmoothie
Copy link
Contributor

Are all your other storybook deps up to date?

@waldemarfm
Copy link
Author

Yes this is happening on a fresh clone of latest, 4.1.0-alpha.9.

@Bobgy
Copy link

Bobgy commented Dec 7, 2018

Hi, I met the same issue. This is a reproduction repo: https://github.com/Bobgy/reproduction-storybook-a11y-bug
It's simply made from a new create-react-app and installing storybook@next
exact version I tested is ^4.1.0-alpha.11. it is still broken

@thebuilder
Copy link
Contributor

The addon-a11y plugin also appears to be broken in the 4.1.1 release now - displaying 0 passes and 0 violations on all stories.

@thebuilder
Copy link
Contributor

thebuilder commented Dec 13, 2018

This is also an issue on the official Storybook example page

I'm guessing this is related to #4704 - Adding a <div id="root-story"> around all my stories, causes aXe to throw a selector warning, but it does run all the tests correctly afterwards. It still scrolls to the bottom of the page.

I'm not sure why the root-story is only added to the Info addon? Scrolling to the bottom of a story is a general issue, that's not just related to pages with the Info addon. The readme addon has the same issue, and in my project i have multiple stories that either render entire pages or components that just take up more than the screen.

index.js:19 Error: Expect axe._selectorData to be set up
    at generateSelector (axe.js:3953)
    at Object.createUniqueSelector [as getSelector] (axe.js:4009)
    at DqElement.get selector [as selector] (axe.js:3400)
    at axe.js:2393
    at Array.map (<anonymous>)
    at axe.js:2386
    at Array.map (<anonymous>)
    at axe.js:2383
    at Array.forEach (<anonymous>)
    at Object.push../node_modules/axe-core/axe.js.helpers.processAggregate (axe.js:2375)

The error is related to #3940 - but it doesn't solve the issue with the plugin not running.

@igor-dv
Copy link
Member

igor-dv commented Dec 13, 2018

@hipstersmoothie, does it work for you after the changes?

@CodeByAlex
Copy link
Member

Having the same issue using Angular

@andria-dev
Copy link

Having the same issue with the following versions:

"react": "^16.7.0",
"react-dom": "^16.7.0"

"@storybook/addon-a11y": "^4.1.4",
"@storybook/react": "^4.1.4"

The accessibility tab always says 0 VIOLATIONS 0 PASSES

@shilman
Copy link
Member

shilman commented Jan 6, 2019

Yippee!! I just released https://github.com/storybooks/storybook/releases/tag/v4.2.0-alpha.10 containing PR #5101 that references this issue. Upgrade today to try it out!

Because it's a pre-release you can find it on the @next NPM tag.

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

@citypaul
Copy link

citypaul commented Jan 19, 2019

I'm still experiencing this issue based on the latest releases:

"react": "^16.7.0",
"react-dom": "^16.7.0",
"@storybook/addon-a11y": "^4.1.7",
"@storybook/addons": "^4.1.7",
"@storybook/react": "^4.1.7"

I'm using the example in the storybook a11y docs, and I'm getting 0 VIOLATIONS 0 PASSES for all stories, even when I copy/paste the example story in your setup docs.

*Edit: Ah, so this hasn't got merged in to the main release branch yet, is that right? At this point in time, that would make 4.1.7 unusable, wouldn't it?

@jtomaszewski
Copy link

Could we include that in 4.1.x release?

clintandrewhall added a commit to elastic/kibana that referenced this issue Feb 15, 2019
## Summary

This PR adds [Storybook](https://storybook.js.org/) to our testing and development suite.

![screen shot 2019-01-21 at 4 35 32 pm](https://user-images.githubusercontent.com/297604/51502196-9f856780-1d9a-11e9-97bf-07c99c3f279b.png)

This will allow us to:

1. create a site outlining all components within Canvas, including their TS type information;
2. demonstrate usage of all components by example;
3. allow for individual component testing, both manually and by Jest;
4. iterate and fix bugs on individual components *without* having to start up Kibana, in a [HMR](https://webpack.js.org/concepts/hot-module-replacement/) environment;
5. automatically generate [snapshots](https://jestjs.io/docs/en/snapshot-testing) based on any examples written;

This PR also converts a few components to Typescript and adds examples.

## How this can help us, (with examples)

I was inspired to add this when I was fixing #25342.  In order to fix my changes, I had to run elasticsearch and kibana, as well as refresh my page whenever I needed to test a change.  Had I had a Storybook instance, I would have been done much faster.

In this PR, you'll see I converted `AdvancedFilter` from `renderers` and `FontPicker` and `ImageUpload` from `public/components`.  Would you believe I discovered and fixed bugs just by converting to Typescript and writing examples?

### `AdvancedFilter`

- `onChange` and `commit` are not marked as required in `propTypes`, but the component will error out if they're not supplied.
- `commit` was actually being called twice when 'Apply' was clicked.  This was shown in the 'Actions' panel when I was testing it.

### `FontPicker`

- The `fonts` collection was not strongly-typed, therefore any string could be passed to the `value` parameter without error.
- While the code allows for any font string to be given to the component, there is no way to currently select that value, nor type it in within the control.  This is likely a bug in design.
- The `aria-labeledby` attribute in the drop down includes `undefined`.  This is likely a bug in EUI:

![screen shot 2019-01-21 at 4 25 58 pm](https://user-images.githubusercontent.com/297604/51501908-5ed91e80-1d99-11e9-913a-ce1bb5f4e352.png)

## How to use

- `cd x-pack/plugins/canvas/`
- Run `node scripts/storybook` to start up a local development version, with HMR.
- Run `node scripts/storybook_build` to build a complete static version of the book.
- Run `node scripts/jest` which will run the Storyshots test; run `node scripts/jest --updateSnapshot` if source code has changed as expected.

## Future Work

- Adding Jest coverage and output to the info panels, ([this](https://www.npmjs.com/package/@storybook/addon-jest) is *sick* functionality).
- Adding automatic [a11y testing](https://www.npmjs.com/package/@storybook/addon-a11y), (currently [blocked](storybookjs/storybook#4889)).
- Adding generic knobs for stories
- Adding more example info, (e.g. who edited last, descriptions, etc).
clintandrewhall added a commit to clintandrewhall/kibana that referenced this issue Apr 2, 2019
This PR adds [Storybook](https://storybook.js.org/) to our testing and development suite.

![screen shot 2019-01-21 at 4 35 32 pm](https://user-images.githubusercontent.com/297604/51502196-9f856780-1d9a-11e9-97bf-07c99c3f279b.png)

This will allow us to:

1. create a site outlining all components within Canvas, including their TS type information;
2. demonstrate usage of all components by example;
3. allow for individual component testing, both manually and by Jest;
4. iterate and fix bugs on individual components *without* having to start up Kibana, in a [HMR](https://webpack.js.org/concepts/hot-module-replacement/) environment;
5. automatically generate [snapshots](https://jestjs.io/docs/en/snapshot-testing) based on any examples written;

This PR also converts a few components to Typescript and adds examples.

I was inspired to add this when I was fixing elastic#25342.  In order to fix my changes, I had to run elasticsearch and kibana, as well as refresh my page whenever I needed to test a change.  Had I had a Storybook instance, I would have been done much faster.

In this PR, you'll see I converted `AdvancedFilter` from `renderers` and `FontPicker` and `ImageUpload` from `public/components`.  Would you believe I discovered and fixed bugs just by converting to Typescript and writing examples?

- `onChange` and `commit` are not marked as required in `propTypes`, but the component will error out if they're not supplied.
- `commit` was actually being called twice when 'Apply' was clicked.  This was shown in the 'Actions' panel when I was testing it.

- The `fonts` collection was not strongly-typed, therefore any string could be passed to the `value` parameter without error.
- While the code allows for any font string to be given to the component, there is no way to currently select that value, nor type it in within the control.  This is likely a bug in design.
- The `aria-labeledby` attribute in the drop down includes `undefined`.  This is likely a bug in EUI:

![screen shot 2019-01-21 at 4 25 58 pm](https://user-images.githubusercontent.com/297604/51501908-5ed91e80-1d99-11e9-913a-ce1bb5f4e352.png)

- `cd x-pack/plugins/canvas/`
- Run `node scripts/storybook` to start up a local development version, with HMR.
- Run `node scripts/storybook_build` to build a complete static version of the book.
- Run `node scripts/jest` which will run the Storyshots test; run `node scripts/jest --updateSnapshot` if source code has changed as expected.

- Adding Jest coverage and output to the info panels, ([this](https://www.npmjs.com/package/@storybook/addon-jest) is *sick* functionality).
- Adding automatic [a11y testing](https://www.npmjs.com/package/@storybook/addon-a11y), (currently [blocked](storybookjs/storybook#4889)).
- Adding generic knobs for stories
- Adding more example info, (e.g. who edited last, descriptions, etc).
clintandrewhall added a commit to elastic/kibana that referenced this issue Apr 2, 2019
* [Canvas] Storybook for testing and development (#29072)

This PR adds [Storybook](https://storybook.js.org/) to our testing and development suite.

![screen shot 2019-01-21 at 4 35 32 pm](https://user-images.githubusercontent.com/297604/51502196-9f856780-1d9a-11e9-97bf-07c99c3f279b.png)

This will allow us to:

1. create a site outlining all components within Canvas, including their TS type information;
2. demonstrate usage of all components by example;
3. allow for individual component testing, both manually and by Jest;
4. iterate and fix bugs on individual components *without* having to start up Kibana, in a [HMR](https://webpack.js.org/concepts/hot-module-replacement/) environment;
5. automatically generate [snapshots](https://jestjs.io/docs/en/snapshot-testing) based on any examples written;

This PR also converts a few components to Typescript and adds examples.

I was inspired to add this when I was fixing #25342.  In order to fix my changes, I had to run elasticsearch and kibana, as well as refresh my page whenever I needed to test a change.  Had I had a Storybook instance, I would have been done much faster.

In this PR, you'll see I converted `AdvancedFilter` from `renderers` and `FontPicker` and `ImageUpload` from `public/components`.  Would you believe I discovered and fixed bugs just by converting to Typescript and writing examples?

- `onChange` and `commit` are not marked as required in `propTypes`, but the component will error out if they're not supplied.
- `commit` was actually being called twice when 'Apply' was clicked.  This was shown in the 'Actions' panel when I was testing it.

- The `fonts` collection was not strongly-typed, therefore any string could be passed to the `value` parameter without error.
- While the code allows for any font string to be given to the component, there is no way to currently select that value, nor type it in within the control.  This is likely a bug in design.
- The `aria-labeledby` attribute in the drop down includes `undefined`.  This is likely a bug in EUI:

![screen shot 2019-01-21 at 4 25 58 pm](https://user-images.githubusercontent.com/297604/51501908-5ed91e80-1d99-11e9-913a-ce1bb5f4e352.png)

- `cd x-pack/plugins/canvas/`
- Run `node scripts/storybook` to start up a local development version, with HMR.
- Run `node scripts/storybook_build` to build a complete static version of the book.
- Run `node scripts/jest` which will run the Storyshots test; run `node scripts/jest --updateSnapshot` if source code has changed as expected.

- Adding Jest coverage and output to the info panels, ([this](https://www.npmjs.com/package/@storybook/addon-jest) is *sick* functionality).
- Adding automatic [a11y testing](https://www.npmjs.com/package/@storybook/addon-a11y), (currently [blocked](storybookjs/storybook#4889)).
- Adding generic knobs for stories
- Adding more example info, (e.g. who edited last, descriptions, etc).

* Update based on errors in CI

* Fix mismerge of dependencies

* Fix yarn.lock as well
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

10 participants