Skip to content
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

Add mouse events to line slices #2128

Merged
merged 1 commit into from
Apr 3, 2023

Conversation

sigmike
Copy link
Contributor

@sigmike sigmike commented Sep 18, 2022

On the Line chart the mouse events (onMouseEnter, onMouseMove, onMouseLeave and onClick) work fine when the mesh is enabled but not when the slices are enabled. This PR fixes that.

Related issue: #1401.

@codesandbox-ci
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit add2bf3:

Sandbox Source
nivo Configuration

@azhard4int
Copy link

@plouc Can we get this merged, we need mouse event feature for line slices at Usermaven.com

@Stormtv
Copy link

Stormtv commented Oct 26, 2022

This would be very helpful

@Stormtv
Copy link

Stormtv commented Oct 31, 2022

@plouc & @wyze any chance to get this merged?

@yo-johnnyd-hc
Copy link

Oh yes please! I need this as well.

@nepstein10
Copy link

Adding on to say this change would be amazing to have merged in!

@zikowang
Copy link

Nice work! We need this feature as well.

@fspin
Copy link

fspin commented Jan 11, 2023

I think this is also related: #2187
It will be great indeed if one of these PR can be merged! 🙏

@un-neil
Copy link

un-neil commented Mar 31, 2023

@plouc & @wyze please can we get this merged?

@WhyBeeKay
Copy link

Bump
Can we get this merged please? @plouc

Copy link
Owner

@plouc plouc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, however I think this should be added to the docs, but this can be done in a separate PR, as adding doc is currently undocumented 😅.

@plouc plouc merged commit 408d46f into plouc:master Apr 3, 2023
@cbix
Copy link

cbix commented Aug 28, 2023

@sigmike @plouc I noticed this passes the slice to the handler while normally they'd receive a point, was this on purpose? It's particularly confusing with TypeScript. While I'm aware the line chart still lacks full TS support I see two possible improvements here:

  1. change the event handler so it optionally receives a slice or a point, something like
export type PointMouseHandler = (point: Point, slice: null, event: React.MouseEvent) => void
export type SliceMouseHandler = (point: null, slice: Slice, event: React.MouseEvent) => void
export interface LineProps {
  // ...
  onMouseEnter?: PointMouseHandler | SliceMouseHandler
  // ...
}
  1. add extra event handlers for slices (onSliceMouseEnter etc.)

@damianr13
Copy link

Is there any fix in version v.0.80.0? I am unable to upgrade due to the following issue: #2310

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.