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 Appointment Interface for Temporals #55

Closed daviddbal closed 8 years ago

daviddbal commented 8 years ago

I changed the name of the default Appointment class from AppointmentImplLocal to AppointmentImplTemporal. This change has not been propagated to the Test folder. If you like it I will change the name elsewhere.

daviddbal commented 8 years ago

I restored AppointmentImplLocal in its original form. All existing code, tests included, should work fine. I am not sure AppointmentImplLocal should stay permanently, though. AppointmentImpTemporal can handle LocalDateTime just as well as AppointmentImplLocal. The only reason AppointmentImplLocal should remain is for legacy purposes.

tbee commented 8 years ago

Legacy purposes is a very valid reason when writing a library. I hope to find time this weekend to take a look (very busy with a JavaFX app that runs on PC, Android and iOS).

daviddbal commented 8 years ago

I made some more changes. The largest addition is an enum to handle the Temporal class conversions.

The changes to Appointment are not as large as it may appear. I think you use tabs and I use spaces so that difference is causing more lines to show up as changed than really are different.

daviddbal commented 8 years ago

Tom,

I know you are busy with designing the app you described. It sounds interesting. I'm impressed that you are getting it on Android. Someday I may try something similar.

I hope to get a decision on the PR soon. Specifically, I want to know about the AppointmentImplTemporal class. I have not uploaded any of my work to Github lately because without the PR the build will fail.

-David

daviddbal commented 8 years ago

I have trouble running the jfxtras test cases on my computer. There are two problems with the clone from Github. First, the source folders are not in the build path on eclipse (I ran Gradle eclipse). I had to add them manually. Adding the build folders manually is not a complete fix. The css files are not copied automatically. I have to copy Agenda.css manually. Second, I get the retrolambda error. I thought I fixed the retrolambda problem, but its back. Oddly, these problems do not occur with jfxtras-labs. With jfxtras-labs the build path in eclipse is correct and there is no retrolambda requirement. Can something be done to make building jfxtras and using it in eclipse simpler?

Therefore, I haven't run the test cases so I don't know for sure my changes cause no problems. However, some test cases in jfxtras-labs that use the changes pass so I am hopeful that no problems will exist.

tbee commented 8 years ago

I understand that you want to PR to be committed. I find myself reluctant because of the potential breaking things. But because I can't sleep anyhow, I'll take a peek now.

daviddbal commented 8 years ago

Committing the PR may not be the best idea. I just want a way to support all the date and date-time based Temporal classes. If you have a better idea then I am willing try it.

tbee commented 8 years ago

There you go :-)

daviddbal commented 8 years ago

thanks. I hope you can sleep now.