Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Tempus-Dominus v6 is added #1002

Merged
merged 13 commits into from
Oct 9, 2023
Merged

Tempus-Dominus v6 is added #1002

merged 13 commits into from
Oct 9, 2023

Conversation

solomax
Copy link
Contributor

@solomax solomax commented Aug 29, 2023 •
edited
Loading

Important changes are:

  • Popper and bootstrap are now separate libraries
  • MomentJs in no more required for Date/Time picker

@martin-g could you please review this one? especially TempusDominus*Config implementation? :)

PS I believe this tempus-dominus-v6 should replace current datepicker-v5, but there are too much changes

@solomax solomax marked this pull request as draft August 31, 2023 06:43
@martin-g
Copy link
Owner

Thanks, @solomax !
I will take a look soon!

@solomax solomax marked this pull request as ready for review September 1, 2023 07:29
@solomax solomax marked this pull request as draft September 1, 2023 11:17
@solomax
Copy link
Contributor Author

solomax commented Sep 1, 2023

fa-EG locale is broken :(

Will add some hacks :)

@solomax
Copy link
Contributor Author

solomax commented Sep 1, 2023

fa-EG locale is broken :(

I'm a bit lost :(

DateTimeFormatter.ofPattern(dateTimePattern).withLocale(locale)

unable to parse date produced by TempusDominus (even after specifying Decimal format)

While this code works as expected:

LocalDate.ofInstant(new SimpleDateFormat("yyyy/M/d", Locale.forLanguageTag("fa")).parse("2003/۱۱/5").toInstant(), ZoneId.systemDefault())

@martin-g Shall Wicket internals be changed? or maybe there are better way? :)

martin-g
martin-g reviewed Sep 4, 2023
View reviewed changes
...c/main/java/de/agilecoders/wicket/core/markup/html/references/PopperJavaScriptReference.java Outdated Show resolved Hide resolved
...icket/extensions/markup/html/bootstrap/form/tempusdominus/AbstractTempusDominusWithIcon.java Outdated Show resolved Hide resolved
...lecoders/wicket/extensions/markup/html/bootstrap/form/tempusdominus/TempusDominusConfig.java Outdated
* @param clazz Class of DateTime object to set restrictions
* @return current instance
*/
public <T> TempusDominusConfig withClass(Class<T> clazz) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should T extend Temporal ? Otherwise the user could pass anything.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately this can be Temporal OR java.util.Date I'm not sure how bounds can be set here :(

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have a use case in your app for using j.u.Date ? If NOT then I'd recommend to support only the new Datetime APIs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hopefully now everything is better :)

...ders/wicket/extensions/markup/html/bootstrap/form/tempusdominus/TempusDominusIconConfig.java Outdated Show resolved Hide resolved
...e/agilecoders/wicket/extensions/markup/html/bootstrap/references/js/wicket.tempus-dominus.js Outdated Show resolved Hide resolved
...strap-samples/src/main/java/de/agilecoders/wicket/samples/pages/TempusDominusPickerPage.java Outdated Show resolved Hide resolved
@martin-g
Copy link
Owner

martin-g commented Sep 4, 2023

Shall Wicket internals be changed? or maybe there are better way? :)

I don't have any experience with Arabic locales and I am not sure what/why does not work.
If you think it is something in Wicket then please start a discussion at dev@wicket.a.o

@martin-g
Copy link
Owner

martin-g commented Sep 4, 2023

PS I believe this tempus-dominus-v6 should replace current datepicker-v5, but there are too much changes

I am fine!
But let's keep the old code as @Deprecated for this major version!

@solomax
Copy link
Contributor Author

solomax commented Sep 12, 2023

Shall Wicket internals be changed? or maybe there are better way? :)

I don't have any experience with Arabic locales and I am not sure what/why does not work. If you think it is something in Wicket then please start a discussion at dev@wicket.a.o

will do :)

@solomax
Copy link
Contributor Author

solomax commented Sep 12, 2023

PS I believe this tempus-dominus-v6 should replace current datepicker-v5, but there are too much changes

I am fine! But let's keep the old code as @Deprecated for this major version!

done :)

@solomax solomax marked this pull request as ready for review September 21, 2023 12:59
@solomax solomax requested a review from martin-g October 6, 2023 09:43
martin-g
martin-g reviewed Oct 6, 2023
View reviewed changes
...e/agilecoders/wicket/extensions/markup/html/bootstrap/references/js/wicket.tempus-dominus.js Outdated
@@ -0,0 +1,19 @@
function createTempusDominus(elementId, config, localization, lang) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO it would be good to add these functions to Wicket.Bootstrap object. Currently they pollute the global space

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed!

@solomax
Copy link
Contributor Author

solomax commented Oct 9, 2023

@martin-g can this version be merged? and (maybe) released? :)))

@martin-g martin-g merged commit a10a7f7 into martin-g:wicket-9.x-bootstrap-5.x Oct 9, 2023
2 checks passed
martin-g pushed a commit that referenced this pull request Oct 9, 2023
@solomax @martin-g
Tempus-Dominus v6 is added (#1002)
afff884 
* Tempus-Dominus v6 is added

* Build should be fixed

* Code clean-up

* Consumers are added to config; Example page is improved

* Dates are being validated onChange

* Jar-based localization seems to be loaded

* OnChangeAjaxBehavior seems to work

* Code comment is added

* Validation errors are being displayed

* tempus-dominus-v5 is marked as deprecated; code clean-up

* Non bundled bootstrap 5.3.2 version is used

* Code related to OnChangeBehavior is simplified

* Comments are addressed

(cherry picked from commit a10a7f7)
@martin-g
Copy link
Owner

martin-g commented Oct 9, 2023

Done!
6.0.5 has been released!
Thank you, @solomax !

@solomax solomax deleted the tempus-dominus-6 branch October 9, 2023 13:31
@solomax
Copy link
Contributor Author

solomax commented Oct 9, 2023

Thanks! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@martin-g martin-g martin-g approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@solomax @martin-g