search

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

Resolving Issue #33 #34

Closed Maxoudela closed 9 years ago

Maxoudela commented 9 years ago

I also fixed two tests failing due to my precedent commit that set a minimum size of 16 to today Button.

tbee commented 9 years ago

Thanks, it works great.

I did changed the test, I felt they were not scripting enough (did not tell a story). And added two for the highlight lists.

Maxoudela commented 9 years ago

Yes I saw no problem it's better!

Also I noticed that they were no name in the copyright, like "" was written. Shouldn't it be replace by Jfxtras somehow?

Also, do you want me to try to implement an abstract class for factorizing some property?

tbee commented 9 years ago

There is a Gradle plugin to put copyright messages in the source code, someone added that quite some time ago. Need to check what is going on with that.

An abstract superclass reduces code duplication, but also forces all three controls to have the same API. It is a question of how you look at them; are they three separate controls who happen to have a similar API (loose coupling), or are they three implementations of the same abstract? Not sure what is wise there. You could do a refactor and see what problems come up and how much is gained. We can always untangle in the future if it turns out loose coupling is better.

Maxoudela commented 9 years ago

Well they will have the same API at first with their basic features (locale, allowNull).

Other things that annoyed me was the fact that they shared the same methods but each name was different : -disabledLocalDates -disabledCalendars -disabledLocalDateTime

I would like to have a single method name so that when I implement a RangeCallBack, I can copy the method and paste it in another component without renaming everything.

But again, this is my personal feeling, maybe it's confusing to have the same API in several components parametrized with different implementations of date.

miho commented 9 years ago

@tbee

There is a Gradle plugin to put copyright messages in the source code, someone added that quite some time ago. Need to check what is going on with that.

If I remember correctly, I only added it to the labs project. The license template .txt has been added to jfxtras though.

tbee commented 9 years ago

Added the license text part to the gradle.build of JFXtras and let it run. All good now.