-
Notifications
You must be signed in to change notification settings - Fork 28
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
feat(TableMessage): Add support for PatternFly tables #425
Conversation
Preview: https://chatbot-pr-chatbot-425.surge.sh A11y report: https://chatbot-pr-chatbot-425-a11y.surge.sh |
e62c8ac
to
73435c1
Compare
28aef09
to
9fc7519
Compare
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.
some suggestions for the table content!
packages/module/patternfly-docs/content/extensions/chatbot/examples/Messages/BotMessage.tsx
Outdated
Show resolved
Hide resolved
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.
looks great!
…mples/Messages/BotMessage.tsx Co-authored-by: Erin Donehoo <[email protected]>
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.
looks good! thanks!
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.
In addition to the file comments below, a non-blocker for this PR would be switching from radio buttons for these examples and using a Select instead -- would reduce the space taken up, but would also allow more unique labeling for the Selects (right now there's basically 2+ radio buttons with the same labels on the page: 2 for "Table" etc). Again non-blocking here if we wanted to open a followup for a docs update.
Opening #433 for Select switch. |
d3477d4
to
23962de
Compare
🎉 This PR is included in version 2.2.0-prerelease.21 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Wire Message to re-route tables to PF tables.
These are just basic tables, with no bells and whistles. I am defaulting to the mobile breakpoint, but we could also do a regular table at certain widths. Let me know what you want to do here - I am just copying Figma right now. I am also letting this be full width, but it will resize down if you change the device viewport or screen size - let me know if that's not what we want.
I am defining a default aria label, allowing passage of aria-labels, and also attempting to generate these based on headers. I have written tests for full tables, blank tables, headerless tables, cell-less tables, 1 column-tables, and 1-cell tables. Let me know if you can break this and we'll add another test.
Doc changes: