-
Notifications
You must be signed in to change notification settings - Fork 375
Rename composable examples #8754
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
Conversation
|
Preview: https://patternfly-react-pr-8754.surge.sh A11y report: https://patternfly-react-pr-8754-a11y.surge.sh |
7181fe4 to
d9871ca
Compare
d9871ca to
3dc50b2
Compare
nicolethoen
left a comment
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 like a duplicate id 'untitled-example' is being used a couple times in the (formerly) composable table docs.
| ## Table examples | ||
|
|
||
| The `TableComposable` component differs from the regular `Table` component, in that it allows you to compose the table by nesting the relevant `Thead`, `Tbody`, `Tr`, `Th` and `Td` components within it. For a less declarative and more implicit approach, use the `Table` component instead. | ||
| This `TableComposable` component differs from the deprecated `Table` component, in that it allows you to compose the table by nesting the relevant `Thead`, `Tbody`, `Tr`, `Th` and `Td` components within it. For a less declarative and more implicit approach, use the `Table` component instead. |
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.
is it still accurate to say "This TableComposable component" rather than "This Table component"? The general bulleted notes below reference the Tablecomposable, too.
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.
@nicolethoen I left these remaining TableComposable references as-is because the <TableComposable /> component itself has not yet been renamed in this PR, but will be as part of #8843 . Using Table here would still be referencing the legacy variant.
Once #8843 is ready for review it will be using all the appropriate references!
jenny-s51
left a comment
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.
Thank you @nicolethoen - investigating the duplicate ID issue...
| ## Table examples | ||
|
|
||
| The `TableComposable` component differs from the regular `Table` component, in that it allows you to compose the table by nesting the relevant `Thead`, `Tbody`, `Tr`, `Th` and `Td` components within it. For a less declarative and more implicit approach, use the `Table` component instead. | ||
| This `TableComposable` component differs from the deprecated `Table` component, in that it allows you to compose the table by nesting the relevant `Thead`, `Tbody`, `Tr`, `Th` and `Td` components within it. For a less declarative and more implicit approach, use the `Table` component instead. |
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.
@nicolethoen I left these remaining TableComposable references as-is because the <TableComposable /> component itself has not yet been renamed in this PR, but will be as part of #8843 . Using Table here would still be referencing the legacy variant.
Once #8843 is ready for review it will be using all the appropriate references!
tlabaj
left a comment
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.
LGTM! Just needs a rebase.
delete examples from legacy dir add todo bump rename TableComposable demos and examples fix filename revert md references for now fix name fix a11y config test update paths revert fix duplicate id revert path update remove whitespace
3cd46c9 to
da88256
Compare
|
Your changes have been released in:
Thanks for your contribution! 🎉 |
What: Closes #8749
Additional issues: Towards #8005.