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.

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

#926 Use Webjars for JQueryUI #927

Conversation

mcr-paulanand
Copy link
Contributor

Resolves #926

Copy link
Contributor

@solomax solomax left a comment

Choose a reason for hiding this comment

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

I would also

  • use *.css from webjars for this base theme wicket-jquery-ui-parent/wicket-jquery-ui-themes/theme-base/src/main/java/com/googlecode/wicket/jquery/ui/theme/Initializer.java
  • WicketWebjars.install(this); should be added to wicket-jquery-ui-parent/wicket-jquery-ui-samples/src/main/java/com/googlecode/wicket/jquery/ui/samples/SampleApplication.java, otherwise examples will not work ....

@solomax solomax merged commit 9004e68 into wicketstuff:master Jun 6, 2024
1 check passed
@solomax
Copy link
Contributor

solomax commented Jun 6, 2024

Hello @martin-g,

It seems this PR breaks samples

Webjars are being used from Initializer from now on

so we got

Caused by: java.lang.IllegalStateException: you have to call WicketWebjars.install() before you can use an IWebjarsResourceReference or any other component.
    at de.agilecoders.wicket.webjars.WicketWebjars.settings (WicketWebjars.java:136)
    at de.agilecoders.wicket.webjars.util.WebjarsVersion$Holder.<clinit> (WebjarsVersion.java:29)
    at de.agilecoders.wicket.webjars.util.WebjarsVersion.useRecent (WebjarsVersion.java:45)
    at de.agilecoders.wicket.webjars.request.resource.WebjarsJavaScriptResourceReference.<init> (WebjarsJavaScriptResourceReference.java:29)
    at org.wicketstuff.jquery.ui.resource.JQueryUIResourceReference.<init> (JQueryUIResourceReference.java:54)
    at org.wicketstuff.jquery.ui.resource.JQueryUIResourceReference.<clinit> (JQueryUIResourceReference.java:37)
    at org.wicketstuff.jquery.ui.settings.JQueryUILibrarySettings.<init> (JQueryUILibrarySettings.java:49)
    at org.wicketstuff.jquery.ui.settings.JQueryUILibrarySettings.get (JQueryUILibrarySettings.java:61)
    at org.wicketstuff.jquery.ui.theme.Initializer.init (Initializer.java:37)
    at org.apache.wicket.Application.initInitializers (Application.java:561)
    at org.apache.wicket.Application.initApplication (Application.java:767)
    at org.apache.wicket.protocol.http.WicketFilter.init (WicketFilter.java:441)

due to WicketWebjars.install(this); is being called in the init() i.e. too late :(

What can be done here?

@martin-g
Copy link
Member

martin-g commented Jun 6, 2024

As error message suggests we need to add 'WicketWebjars.install()' to the respective WebApplication#init()

@martin-g
Copy link
Member

martin-g commented Jun 6, 2024

Oh, now I see the text after the stacktrace.
I think it should be OK to call it in an initializer too.

@solomax
Copy link
Contributor

solomax commented Jun 7, 2024

Works as expected!

Thanks a million @martin-g :)

@mcr-paulanand mcr-paulanand deleted the refactoring/#926-use-webjars-for-jquery-ui branch June 7, 2024 02:07
@reckart
Copy link
Contributor

reckart commented Jun 7, 2024

due to WicketWebjars.install(this); is being called in the init() i.e. too late :(

@solomax so how did you solve this?

@reckart
Copy link
Contributor

reckart commented Jun 7, 2024

I think this change needs to be made in the initializer of all of the themes separately.

@solomax
Copy link
Contributor

solomax commented Jun 7, 2024

I think this change needs to be made in the initializer of all of the themes separately.

I also thought so

But I believe right now it's not necessary due to the fact the only theme uses webjar so far :)

@mcr-paulanand mcr-paulanand restored the refactoring/#926-use-webjars-for-jquery-ui branch June 7, 2024 08:53
@mcr-paulanand mcr-paulanand deleted the refactoring/#926-use-webjars-for-jquery-ui branch June 7, 2024 09:19
@mcr-paulanand
Copy link
Contributor Author

I think this change needs to be made in the initializer of all of the themes separately.

I also thought so

But I believe right now it's not necessary due to the fact the only theme uses webjar so far :)

I have created a PR #934 to resolve this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use webjars for JQuery UI
4 participants