-
Notifications
You must be signed in to change notification settings - Fork 358
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
Charts - fix broken tooltip examples for v5 #8592
Conversation
Preview: https://patternfly-react-pr-8592.surge.sh A11y report: https://patternfly-react-pr-8592-a11y.surge.sh |
…new div tag in the SVG patternfly#8591
1ed8144
to
d2d6a30
Compare
@@ -297,7 +297,7 @@ This demonstrates how to embed HTML within a tooltip. Combining cursor and voron | |||
|
|||
```js | |||
import React from 'react'; | |||
import { Chart, ChartArea, ChartAxis, ChartCursorFlyout, ChartCursorTooltip, ChartGroup, ChartPoint, ChartThemeColor } from '@patternfly/react-charts'; | |||
import { Chart, ChartArea, ChartAxis, ChartCursorFlyout, ChartCursorTooltip, ChartGroup, ChartPoint, ChartThemeColor, createContainer } from '@patternfly/react-charts'; |
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 createContainer is an unnecessary unused import
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.
It's used just below on line 315. I noticed it because CodeSandbox fails to load the example without it.
This will also likely need a code mod, right? To at least warn users that are using tooltips in charts that they need to wrap them in foreignObjects? |
This particular example is more to show how to create a custom label component. However, I've created the codemod issue below. |
Need to fix two broken legend tooltip examples due to the Tooltip popper outputting a new div tag.
#8591