-
Notifications
You must be signed in to change notification settings - Fork 352
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(NotificationDrawer): demo conversion from JS to TS #9516
Conversation
Preview: https://patternfly-react-pr-9516.surge.sh A11y report: https://patternfly-react-pr-9516-a11y.surge.sh |
I cannot figure out why the ts -> js button is not appearing in the second demo and the site doesn't seem to think the second demo is written in ts... still looking into it, but maybe someone else here knows? |
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.
Haven't looked into the above issue that Nicole mentioned, but I'd imagine we could update the js file=...
to ts file=...
in the MD file. Do you get the same issue as the Nav demo PR when doing so?
packages/react-core/src/demos/NotificationDrawer/examples/NotificationDrawerGrouped.tsx
Outdated
Show resolved
Hide resolved
packages/react-core/src/demos/NotificationDrawer/examples/NotificationDrawerBasic.tsx
Outdated
Show resolved
Hide resolved
packages/react-core/src/demos/NotificationDrawer/examples/NotificationDrawerBasic.tsx
Outdated
Show resolved
Hide resolved
@nicolethoen I wonder if it's just an issue with the preview... I tried editing my Org clone locally to add in the tsx example files to react-core (using |
Maybe a rebase/rebuild would iron out the surge? |
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 its all working now!
Your changes have been released in:
Thanks for your contribution! 🎉 |
What: Closes #9438