-
Notifications
You must be signed in to change notification settings - Fork 376
Create "deprecated" subpaths in react-table #8362
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-8362.surge.sh A11y report: https://patternfly-react-pr-8362-a11y.surge.sh |
| sourceMD(path.join(reactTablePath, '/**/demos/*.md'), 'react-demos'); | ||
| sourceMD(path.join(reactTablePath, '/**/TableComposable/examples/*.md'), 'react'); | ||
| sourceMD(path.join(reactTablePath, '/**/Table/examples/*.md'), 'react-legacy'); | ||
| sourceMD(path.join(reactTablePath, '/deprecated/components/**/examples/*.md'), 'react-deprecated'); |
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.
Unfortunately, this will require some changes in documentation-framework to work properly. According to the issue this PR is linked to, it looks like the expectation is for there to be a React deprecated tab, which currently the doc framework does not account for.
react-next is a keyword leveraged within the doc framework to distinguish between identically named components. The same will have to be done for deprecated.
patternfly/patternfly-org#3126
If this line is to be included in this PR, I would expect changes in patternfly-org's doc framework to be added/merged first.
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.
I can get a PR up for that.
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.
So the build in @nicolethoen's PR will continue to fail because the path for /deprecated is not setup yet (other parts of this PR). So I would suggest commenting this line out with a TODO (maybe paste the PR link above in that comment) or removing this line altogether until patternfly/patternfly-org#3291 is reviewed and merged.
This way we can merge this PR without false expectations, get Nicole's build to pass, get that merged, and come back to this line and uncomment/add it back without issue.
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 for clarifying @jeffpuzzo - that sounds like a good plan. Updated to reflect your suggestion.
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.
I think this will need to be merged into the main branch if it is needed to be incorporated on org.
Then it can be duplicated in v5 or we can rebase v5 to include this update so you can get unstuck on the branch work.
What: Closes #8360
Additional issues: #7823