Skip to content

Adding ability to show label on hovering the track#69

Open
oyeanuj wants to merge 2 commits intodavidchin:masterfrom
oyeanuj:show-label-on-hover
Open

Adding ability to show label on hovering the track#69
oyeanuj wants to merge 2 commits intodavidchin:masterfrom
oyeanuj:show-label-on-hover

Conversation

@oyeanuj
Copy link
Copy Markdown
Contributor

@oyeanuj oyeanuj commented Mar 10, 2017

Hi @davidchin! Back again!

This is another feature that I found myself wanting - an ability to hover on the track and see the value at that position, if one was to click. I am using this library as a slider for a media player, and it is quite an expected behavior in a media player to be able to view the position at certain point to move accurately.

For the PR, I've followed the conventions that you already have in the library. Most of the functionality is there but the only thing I am struggling with is setting the css for the label appropriately based on the event. I imagine this is something you are more adept than I am, so any help would be much appreciated.

If this looks good, and works, then I'll be happy to add docs, tests, et al.

Thank you!

oyeanuj added 2 commits March 9, 2017 18:23
- adds the event listener,
- calculates position and value
- adds node for Label
@davidchin
Copy link
Copy Markdown
Owner

Hey @oyeanuj, at this point, I'm not 100% sure if I want to have a hover label in this library. To me, it feels very specific to your use case (media player) and your design, if you know what I mean.

I think a better approach might be to either:

  1. Build a decorator component in your app to add additional functionality specific to your needs.
  2. Or to modify this component so that it can be composed with a custom Track and Slider (something like make more customizable #44), making it more extensible as a result.

@oyeanuj
Copy link
Copy Markdown
Contributor Author

oyeanuj commented Apr 18, 2017

@davidchin I like the second solution. With #44 merged, I can look into nice patterns to decorate the same.

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.

2 participants