-
Notifications
You must be signed in to change notification settings - Fork 26
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
Custom parameters prefix knotx snippet #419
Custom parameters prefix knotx snippet #419
Conversation
…gs' into feature/custom-parameters-prefix-knotx-snippet
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Type parameter was required for script tag. Is it still required when non script tag is used? / knotx:snippet?
@pun-ky thanks for this catch, of course it is not required. |
* Default constructor | ||
*/ | ||
public SnippetOptions() { | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about default values ? a good practice is to have private init() method invoked in default constructor and inside jsonobject constructor (before converter). an init method will simply set default values for tag and prefix. that way we avoid potential nullpointers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right. I first thought that there will be no default values, but this is very good place to have those.
…-parameters-prefix-knotx-snippet
…-parameters-prefix-knotx-snippet
Description
Snippet parameters names are too long, because of being HTML5 compliant (as the snippet tag is
<script>
). Since Knot.x1.2.1
, it is possible to customize snippet tag name (see #385 for more details).This enchancment is followup of this discusion.
Motivation and Context
Types of changes
Checklist:
I hereby agree to the terms of the Knot.x Contributor License Agreement.