-
Notifications
You must be signed in to change notification settings - Fork 14
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
chore(grid): refactor storybook #427
Conversation
const { | ||
role, | ||
'aria-selected': selected, // the gridcell itself is not selectable in this implementation | ||
...props | ||
} = getGridCellProps({ rowIdx, colIdx }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This prop extraction and subsequent usage of the current grid's selection
concept below is a key demo differentiation that corrects previous invalid ARIA and points the way for how we'll address an updated ColorSwatch
in react-components
by refactoring to use radio buttons (or checkboxes, depending on design) for selection.
"jsx-a11y/label-has-associated-control": "off", | ||
"react/button-has-type": "off", | ||
"react/jsx-key": "off" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any particular reasons why these rules are turned off?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They weren't really turned off, per se. This limits their scope to just spec tests. Previously they were applied to both stories and spec tests.
size: number; | ||
} | ||
|
||
export const GridStory: Story<IArgs> = ({ as, ...props }) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stories should not internally mask gaps in the underlying component code. In this case, the idPrefix
is an optional prop that can be set via storybook controls. The undefined
string is an indication that either a) the prop should be mandatory or b) there should be a default fallback for ID generation (preferable and similar to other container code). This should probably be addressed as a follow-on.
Description
Following #420, update storybook for the
grid
package.Checklist
yarn start
)analyzed via axe and evaluated using VoiceOverincludes new unit teststested in Chrome, Firefox, Safari, Edge, and IE11