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

WICKET-7066: Make type=module possible for JavascriptHeaderItem #598

Merged
merged 3 commits into from
Sep 12, 2023

Conversation

hosea
Copy link

@hosea hosea commented Aug 4, 2023

No description provided.

@@ -16,7 +16,10 @@
*/
package org.apache.wicket.markup.head;

import static org.apache.wicket.markup.head.JavascriptReferenceType.*;
Copy link
Member

Choose a reason for hiding this comment

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

No wildcard imports please!

Copy link
Author

Choose a reason for hiding this comment

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

PR adjusted


public enum JavascriptReferenceType {

TEXT_JAVASCRIPT("text/javascript"), MODULE("module");
Copy link
Member

Choose a reason for hiding this comment

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

What about other types like importmap ?
Using an enum makes it hard to use custom values when/if needed.

Copy link
Author

Choose a reason for hiding this comment

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

Hi Martin,

thank you for your remark.
I just started with the approach using a simple String instead of an Enum.
But then I realized, that there are only two "real" types for Javascript-References: Javascript-Mimetype ( "text/javascript", "application/javascript", "application/ecmascript") and "module".
All other types imply that the src-Attribute is ignored and the "real content" is placed inside the body of the <script>-Tag. But this is not the intention of the AbstractJavaScriptReferenceHeaderItem.
The documentation I used: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/script/type

Kind regards
Hans

*/
package org.apache.wicket.markup.head;

public enum JavascriptReferenceType {
Copy link
Member

Choose a reason for hiding this comment

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

Public classes need javadoc.

Copy link
Author

Choose a reason for hiding this comment

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

PR adjusted

Schäfer, H.H. (Hans Hosea) and others added 2 commits August 9, 2023 15:45
@martin-g
Copy link
Member

@hosea Please review 74943c1

@bitstorm
Copy link
Contributor

@hosea ping! :-)

@hosea
Copy link
Author

hosea commented Sep 12, 2023

@hosea ping! :-)

Hi @bitstorm : I reviewed Martin's work and he also replied. Please have a look at it and give your opinion.

Copy link
Contributor

@bitstorm bitstorm left a comment

Choose a reason for hiding this comment

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

Thanks you!

@bitstorm bitstorm merged commit 98bddc8 into apache:wicket-9.x Sep 12, 2023
1 check passed
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.

3 participants