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

Added onclick pointer to bar chart/histogram #227

Merged
merged 4 commits into from
Nov 25, 2024
Merged

Conversation

SanjeevLakhwani
Copy link
Contributor

This was removed when onClick started to work with on click on the possible render area for a bar as it caused multiple complications. Now it added back but only for bars even though you are allowed to click around it also.

@SanjeevLakhwani SanjeevLakhwani requested a review from gsfk November 21, 2024 20:01
@SanjeevLakhwani SanjeevLakhwani self-assigned this Nov 21, 2024
Copy link
Member

@gsfk gsfk left a comment

Choose a reason for hiding this comment

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

Bit of a conflict between actual clickable area (entire height of bar area) and area where you get the pointing hand cursor (filled-in portion of the bar). But I'm guessing this is a hassle to fix?

@davidlougheed
Copy link
Member

There's a way to add a cursor: pointer to the whole thing, but that ends up overriding hovers without click too. Not sure if there's a simple solution but this does help a bit.

Copy link
Member

@davidlougheed davidlougheed left a comment

Choose a reason for hiding this comment

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

can you add a comment explaining the noop usage, since it's a bit unclear to a newcomer to the code?

Copy link
Member

@davidlougheed davidlougheed left a comment

Choose a reason for hiding this comment

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

lgtm

@SanjeevLakhwani SanjeevLakhwani merged commit 68bb886 into main Nov 25, 2024
2 checks passed
@SanjeevLakhwani SanjeevLakhwani deleted the chart-pointer branch November 25, 2024 15:42
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.

3 participants