elastic-charts
elastic-charts copied to clipboard
feat(tooltip): pinned tooltip with actions
Summary
Adds sticky tooltip interaction whenever Tooltip.actions
are defined. See demo story here.
Features:
- Right-click to stick tooltip
- Select series on stuck tooltip
- Disables stickiness for empty tooltips
- trigger stickiness from tooltip actions or flag
- trigger actions with selected series
- global click listener to dismiss pinned tooltip
- Clear pinned tooltip on resize, esc or outside click
- limit scroll height to 5 items
- scroll nearest elements into view
Chart types with context menu:
- [x]
xy_chart
(bar) - story example - [x]
xy_chart
(line) - story example - [x]
parition_chart
- story example - [x]
heatmap
- story example - [x]
flame_chart
- story example
TODOs
- Configure what is passed back to user (heatmap & flame)
- Block tooltip render when brushing (xy)
https://user-images.githubusercontent.com/19007109/185769263-d0e4b4c1-ffb4-49e4-9963-0944c74e2cb1.mp4
Testing in kibana
To test against this PR on kibana update the package.json
to point to the tooltip-actions-dist
branch.
// package.json
{
"dependencies": {
"@elastic/charts": "https://gitpkg.now.sh/nickofthyme/elastic-charts/packages/charts?tooltip-actions-dist"
}
}
Then run bootstrap with --no-validate
flag...
yarn kbn bootstrap --no-validate
See kibana example/testing/demo at https://github.com/elastic/kibana/pull/141503
Details
Modifies the interactions
redux state to add tooltip stuck/selection state and account for the stuck state and disable existing interactions like drag and click while stuck.
The esc option to cancel the brush interaction was added a while ago but was a little loose when adding and removing the keyboard event listeners.
I have noticed that some of the tooltip interactions are tightly coupled to the redux store/selectors. This might become problematic as we are adding event listeners within
packages/charts/src/components/chart_container.tsx
that may not be implemented inside the reducers. For example, we had akeyup
listener that was added but the reducer that it triggers may not actually do any logic.We should have a better way to communicate interactions to the component that does not require as much redundant checks or logic.
Scroll into view demo
https://user-images.githubusercontent.com/19007109/187324616-175be9cb-d09c-40e1-bde0-3c2341313983.mp4
This feature works by scrolling to the first highlighted tooltip value. Some improvements on this logic could be...
- Some type of collapsing on groups for charts with hierarchical structures to always show highlighted values.
- Collapse values in between until tooltip is pinned.
- Currently the order of tooltip items is arbitrary but if we change this to be nearest-first we could have a better interaction. Currently, the closest is not element is not always scrolled to view, as seen at the end of the recording above where the cursor is closest to the line point but the bar values is focused.
- Shuffle the items to allow showing all highlighted values. Might be a bad idea
Checklist
- [x] The proper chart type label has been added (e.g.
:xy
,:partition
) - [x] The proper feature labels have been added (e.g.
:interactions
,:axis
) - [ ] The
:theme
label has been added and the@elastic/eui-design
team has been pinged when there areTheme
API changes - [ ] All related issues have been linked (i.e.
closes #123
,fixes #123
) - [ ] New public API exports have been added to
packages/charts/src/index.ts
- [ ] Unit tests have been added or updated to match the most common scenarios
- [ ] The proper documentation and/or storybook story has been added or updated
- [ ] The code has been checked for cross-browser compatibility (Chrome, Firefox, Safari, Edge)
- [ ] Visual changes have been tested with all available themes including
dark
,light
,eui-dark
&eui-light
Agree with Robert, this only partially is related to the tooltip, this is more a contextual menu than a sticky/pinned tooltip.
For the interaction/cancellation/clearing it should behave exactly as a right-click context menu in the browser: if you click elsewhere or in one item of the menu, the menu will disappear immediately
It seems like if there is a custom tooltip bit, it won't render the "right click to stick" message anymore:
It still works though:
Maybe I'm using it wrong
@flash1293 Nope you are correct, I am working on adding support for the customTooltip
option.
Not sure whether this is some kind of bug with my way of loading this branch, but the actions are not typed:
I tested a bit more and the current api is a bit limiting. There can only be one set of actions for a chart no matter what value the user clicked. This is an assumption that holds true most of the time, but not necessarily always.
Also due to this setup it's not possible to only fetch some information if the user is actually opening the tooltip which might be necessary at some point (we could work around it right now, but not forever) - for example we might want to disable an action if a certain saved object is created already, but we need to check with an additional request. We can only send this request if the user actually pinned the tooltip - right now there is no place to do this
What I'm suggesting is instead of a prop which takes a list of actions, having a prop which takes a component to render the actions on its own
e.g. like this: old:
actions: {[ label: "abc", onSelect: (values) => handleClick(values) ]}
new:
import { TooltipAction } from '@elastic/charts';
function TooltipActions(values) {
useEffect(() => {
// some async stuff
})
return <>
<TooltipAction label="abc" onClick={() => handleClick(values)}>
<>
}
// ...
actions: {TooltipActions}
it will also come in handy later for having more sophisticated interactivity in there instead of just a list of links to click (I remember some mockups with mini-forms in there).
Let me know what you think.
Deployments - 912185fdda7649464959c0e9e999ac13d09fe3fb
@monfera I'm getting an issue where pinning the tooltip then quickly clicking outside of the chart, causes the chart to go blank on the next render and following renders.
I believe it is somehow related to contextRestoreHandler
specifically the code in the setTimeout
below. Increasing this timeout makes it easier to trigger the blank chart (i.e. time to pin and blur chart). Any thoughts on why this maybe the case?
https://github.com/elastic/elastic-charts/blob/accb50a790c1785c8b691e66f52587a88611dd66/packages/charts/src/chart_types/flame_chart/flame_chart.tsx#L1192-L1195
@monfera I'm getting an issue where pinning the tooltip then quickly clicking outside of the chart, causes the chart to go blank on the next render and following renders.
Hey @nickofthyme this https://github.com/elastic/elastic-charts/pull/1782/commits/98f5436fafacba186c1a582b53f89b1e1816c045 should solve the issue. You where registering a mousemove listener when handling the context menu click. This cause the handleMouseDragMove
to be called making the glitch to appear. You can better reproduce the problem by right-click on a node and by dragging outside the context menu.
@gvnmagni These changes are nearly code complete, just fixing up the code checks and doing a final check on screenshots. Please take a look at let me know if there are any changes you'd like to make. I am missing a few colors for the dark mode namely the tooltip hover highlighter color and the action text color. If they are the same that's fine just want to make sure as the figma colors don't match any of the eui color values here.
buildkite update vrt
Thank you Nick! I believe @markov00 is already working on both colors and will be fixed soon!
buildkite update vrt
buildkite update vrt
Deployments - f4d0656
The changes on the VRTs from master looks good to me and only related to:
- the added tooltip border
- a tooltip rendered with the prompt if the list of values where too long
buildkite update vrt
Everything looks good expect for the fact that the
control+click
on mac doesn't works as expected. It initially pins the tooltip but a subsequent mouse movement will unpin the tooltip.control+click
should follow the same behaviour of thesecondary click
on mac (usually the two finger click)
Good catch, fixed in 0367fed