-
Notifications
You must be signed in to change notification settings - Fork 164
Fixes issue #1252: bootstrap-confirmation client-side events are fixe… #1253
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
Fixes issue #1252: bootstrap-confirmation client-side events are fixe… #1253
Conversation
…are fixed; bootstrap-confirmation.js code clean-up; Examples are updated
| target.add(feedback); | ||
| } | ||
| }; | ||
| /*confirmationButton.add(new AjaxEventBehavior("confirmed.bs.confirmation") { |
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.
Hello @martin-g,
I probably found an issue in Wicket
for whatever reason this behavior is NOT working if being set before ConfirmationBehavior)
and works as expected if being set after ....
I was unable to find the reason and would appreciate any help with it :)))
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.
Hi @solomax !
Sorry! I don't have the time to help you with this.
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.
I don't think it is a Wicket problem. If I see this correctly, AjaxEventBehavior and the ConfirmationBehavior both register a listener for "confirmed.bs.confirmation", so it is probably an issue boiling down to the order in which the event listeners are executed in the browser. I didn't check what exactly the listeners do though.
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.
Correction: bootstrap-confimration.js doesn't listen for the event but trigger it. Still, are you sure it's not a client-side JS issue?
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.
@reckart I wasn't able to find the root cause :((( tried to debug but with no luck :((
I'm also not sure it is wicket problem, but I can see no JS conflict, would appreciate if you can take a look :))
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.
Since this potential issue is not affecting anyone else, maybe we can merge and release while I'll try to further investigate it in my spare time, WDYT? :))
|
@solomax Should I merge as-is for this release ? |
|
@martin-g yes, please :) |
…d; bootstrap-confirmation.js code clean-up; Examples are updated