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

Button binding within buttonset binding breaks on destroy domNodeDisposal #25

Open
haydenc opened this issue Feb 10, 2014 · 5 comments
Open

Comments

@haydenc
Copy link

haydenc commented Feb 10, 2014

Minor issue - jQuery buttonset destroy takes care of destroying the buttons within it, but when

$(element)[widgetName]('destroy');

is called for the button binding itself jquery will throw the following error "cannot call methods on button prior to initialization; attempted to call method 'destroy'"

@gvas
Copy link
Owner

gvas commented Feb 11, 2014

@haydenc: Could you create a fiddle demonstrating the bug? Here's my version (which doesn't throw any error): http://jsfiddle.net/gvas/urLfb/
Thanks!

@vgaiduk
Copy link

vgaiduk commented Jan 16, 2015

Please see http://jsfiddle.net/urLfb/2/

Sometimes there is a need to define bindings for both buttons (to set the options) and the enclosing buttonset (to visually unite the buttons)

@gvas
Copy link
Owner

gvas commented Jan 16, 2015

Aha, now I see the problem, thanks.

I have to decide what should I do with this/should I take any actions. Meanwhile, if the only reason to use the buttonset widget is to modify the buttons' visual style, you can apply the ui-buttonset css class to the enclosing element: http://jsfiddle.net/urLfb/3/

@vgaiduk
Copy link

vgaiduk commented Jan 16, 2015

Thank you for a nice workaround - it does most of the task.
However, not all of it (e.g. it rounds the corners of all buttons instead of doing it for external buttons only) but we can live without it.

May be it is worth fixing it to be on a clean side? It shall be somewhere in the knockout-jquery logic, since jqueryui works by itself without throwing exceptions, if you apply button and buttonset in JavaScript instead of data-bind, please see: http://jsfiddle.net/urLfb/4/

@bago
Copy link
Collaborator

bago commented Mar 6, 2015

Same issue here, but not "minor": my buttonset+button bindings are inside an "if" binding so they are added/disposed frequently and this bug break it.

Applying the classes manually doesn't work too unless I also apply the corner classes (and I can't do that.. it's easier to patch ko-jqui).

jQuery doc states that "buttonset destroy" also destry inner buttons.

Until you find a better (cleaner) options, I guess this would be an improvement:

try {
  $(element)[widgetName]('destroy');
} catch (e) {  } 

If you want to narrow it further it could be catched only for "widgetName == 'button'"

A "cleaner" option would be to have buttonset store something in the bindingContext so that inner buttons can detect they are nested in a buttonset and skip calling the destroy method, but I'm not sure it worth it (the catch is much easier).

ghost pushed a commit to firedrum-marketing/knockout-jqueryui that referenced this issue Apr 4, 2017
…oy domNodeDisposal.

Signed-off-by: Jonathan Horowitz <[email protected]>
ghost pushed a commit to firedrum-marketing/knockout-jqueryui that referenced this issue Apr 4, 2017
…oy domNodeDisposal.

Signed-off-by: Jonathan Horowitz <[email protected]>
ghost pushed a commit to firedrum-marketing/knockout-jqueryui that referenced this issue Apr 6, 2017
…oy domNodeDisposal.

Signed-off-by: Jonathan Horowitz <[email protected]>
ghost pushed a commit to firedrum-marketing/knockout-jqueryui that referenced this issue Apr 6, 2017
…oy domNodeDisposal.

Signed-off-by: Jonathan Horowitz <[email protected]>
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

No branches or pull requests

4 participants