search

JFXtras / jfxtras-styles

Java, JavaFX themes or look and feels. Currently contains JMetro theme.
https://pixelduke.com/java-javafx-theme-jmetro/
642 stars 144 forks source link

Fixes #193: Slider - Unable to update slider by clicking on the track before the thumb #217

Closed cansik closed 2 years ago

cansik commented 2 years ago

This fixes #193 by propagating all the events of the fill down to the track, instead of just calling the same event handler for the popup.

satsen commented 2 years ago

It would be cleaner and more logical to do fill.eventDispatcherProperty().bind(track.eventDispatcherProperty()); instead.

dukke commented 2 years ago

Hi @cansik

Truly sorry for the late reply.

I think the fix is sound.

I'm planning on removing the skin code from JMetro. It will start using fxskins. Could you possibly file a PR like this in fxskins so your contribution (and the fact that you contributed) can be registered there?

Thanks and sorry again..

dukke commented 2 years ago

I'm also merging this into jfxtras-styles. Doesn't hurt to have this there until jmetro starts using fxskins. At least the bug will be fill till this happens.

Thank you very much @cansik (Florian)

satsen commented 2 years ago

@dukke Am I mistaken or is using a property bind not better here? (my comment https://github.com/JFXtras/jfxtras-styles/pull/217#issuecomment-1098204844)

dukke commented 2 years ago

It's not a big difference since the fill object is only used in SliderSkin and it's very unlikely we will be switching event dispatchers of fill inside SliderSkin.

Anyway this code will disappear soon as JMetro will start using FXSkins for all the Skins. In FXSkins we do that binding.

satsen commented 2 years ago

@dukke Alright, looking forward to seeing this useful decoupling :+1: