docs(Table): add Editable Table example #10341
Conversation
|
Preview: https://patternfly-react-pr-10341.surge.sh A11y report: https://patternfly-react-pr-10341-a11y.surge.sh |
adamviktora
force-pushed
the
table-editable-example
branch
from
a67cea7 to
acd45ff
Compare
adamviktora
requested review from
mturley,
a team,
wise-king-sullyman and
nicolethoen
and removed request for
a team
There was a problem hiding this comment.
Looks like the example is erroring out for some reason on the surge build, but it works locally, not sure what's going on there.
What's the deal with importing each react-core component from dist individually? I tried changing that to a normal import locally and it didn't seem to cause any issues.
I do also wonder if this should be a demo instead of an example with how many react-core components it's using 🤔
Other than those questions though this looks awesome 🚀
|
The example error in the surge build is probably because the surge build is old (I then force pushed another commit), and I was using a generic component in the previous commit and it could not parse the syntax. About the individual imports from dist, I at first thought we should import each file individually when importing to another package, the same way we import icons (I read this article from Martin, but I misunderstood it). |
There was a problem hiding this comment.
Couple of comments below. Additionally, there's some markup issues with the table itself:
- There needs to be a
Thcomponent for the editable buttons column. If we want to keep it blank, then screenreadertext needs to be supplied such as "Row edit actions". See the table with actiosn example - The edit buttons (edit, save, close) need to be wrapped in a
Td. Right now thedivwrappers are rendered as chlldren totr
Both of the above issues cause the edit buttons not to be navigable via screen reader since semantically there's no valid table cells to navigate to.
| <div className={css(inlineEditStyles.inlineEditGroup, inlineEditStyles.modifiers.iconGroup, 'pf-m-action-group')}> | ||
| <div className={css(inlineEditStyles.inlineEditAction, inlineEditStyles.modifiers.valid)}> | ||
| <Button | ||
| aria-label={saveAriaLabel} |
There was a problem hiding this comment.
For this, the cancel button, and the edit button, we should have unique aria-labels for each row. E.g. "Edit row 1" or something
| dataLabel={columnNames.textInput} | ||
| staticValue={data.textInput} | ||
| editingValue={ | ||
| <TextInput |
There was a problem hiding this comment.
These textinputs should have an aria-label
| <EditableCell | ||
| dataLabel={columnNames.checkboxes} | ||
| staticValue={data.checkboxes.join(', ')} | ||
| editingValue={dataOptions?.checkboxes.map((option) => { |
There was a problem hiding this comment.
Only other thing is that the checkboxes and radios for each row should be wrapped in a pf-v5-c-inline-edit__group pf-m-column element, with the checkbox group having role="group" and radios having role="radiogroup". Each should also have an aria-label to the group container. Core markup (core needs to be updated to add the role and aria label to the checkbox group):
|
Your changes have been released in:
Thanks for your contribution! 🎉 |
What: Closes #6597