JFXtras / jfxtras

A supporting library for JavaFX, containing helper classes, extended layouts, controls and other interesting widgets.
http://jfxtras.org
Other
599 stars 123 forks source link

new getNodeForPopup method and more #53

Closed daviddbal closed 8 years ago

daviddbal commented 8 years ago

Tom,

This PR contains several commits that do the following:

The appointmentBodyPaneMap I added in the second commit has been removed.

-David

tbee commented 8 years ago

Where do you need the oneAppointmentCallback for? De appointments observable collection should provide the same info?

daviddbal commented 8 years ago

That's a good point. I can listen to changes to selectedAppointments, if it contains only one then I can do what ever I want. Is that what you mean?

tbee commented 8 years ago

yeah

daviddbal commented 8 years ago

I made the change. The selecte one appointment callback is gone.

daviddbal commented 8 years ago

Tom,

I changed the appointmentNodeMap to use System.identityHashCode(a). I added a clean-up listener also. The feature passes simple manual testing. I didn't add any automatic tests.

I also rebased.

Please take a look.

-David

daviddbal commented 8 years ago

Are the equals and hashcode methods staying or being removed?

tbee commented 8 years ago

Good question. On the one hand I'm a big fan of not having code that has no purpose, on the other hand, given that user may implement their own equals, it is good to have them in place to test against. But since Agenda must render two totally identical appointments separately, I think they should be removed.

daviddbal commented 8 years ago

Maybe they should be removed from Agenda, but put in a separate test-only Appointment implementation. That way some tests can be made to guarantee correct behavior in case a user implements another equals.