Opened 7 years ago

Closed 4 years ago

#15938 closed defect (fixed)

Can't cancel BusyButton in onClick handler

Reported by: Gavin R Owned by: dylan
Priority: undecided Milestone: 1.11
Component: DojoX Form Version: 1.8.0
Keywords: Cc:
Blocked By: Blocking:

Description

Attempts to cancel() a BusyButton? in a click handler for a dojox.form.BusyButton? do not work because this.makeBusy() seems to be called _After_ the click handler in the 1.7.3 code (and 1.8).

Attachments (1)

15938.patch (1.0 KB) - added by Nick Fenwick 7 years ago.
Suggested patch to BusyButton?.js. I got bogged down on the tests page doing an AMD conversion, the BusyButton?.js file needs breaking into many smaller modules.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 7 years ago by Dspriggs

I'm having the same issue. This would fix it I think:

change this current code:

_onClick: function(e){

summary: on button click the button state gets changed

only do something if button is not busy if(!this.isBusy){

this.inherited(arguments); calls onClick()

this.makeBusy();

}

}

to this:

_onClick: function(e){

summary: on button click the button state gets changed

only do something if button is not busy if(!this.isBusy){

this.makeBusy();

this.inherited(arguments); calls onClick()

}

}

comment:2 Changed 7 years ago by Nick Fenwick

Moving the makeBusy() call to before this.inherited() does not work. makeBusy() sets the button to disabled, and then inside inherited() _ButtonMixin's onClick handler, that would invoke your onClick hander (that would then cancel() the busy button) exits early because it checks this.disabled.

I'll attach a suggested patch.

Changed 7 years ago by Nick Fenwick

Attachment: 15938.patch added

Suggested patch to BusyButton?.js. I got bogged down on the tests page doing an AMD conversion, the BusyButton?.js file needs breaking into many smaller modules.

comment:3 Changed 6 years ago by Gavin R

Any word on this getting into the next version of Dojo? I thought it would get into 1.8 but it's still broken in that version for us.

comment:4 Changed 6 years ago by Kitson Kelly

Owner: changed from dante to Kitson Kelly
Status: newassigned

comment:5 Changed 4 years ago by Gavin R

I've verified that this patch seems to fix my issue. What else needs to happen on this for it to be integrated?

comment:6 Changed 4 years ago by dylan

Milestone: tbd1.11
Owner: changed from Kitson Kelly to dylan

@gavinr, if you can create a PR, I'll review and land it this week. Thanks!

comment:7 Changed 4 years ago by Gavin R

Thanks dylan. PR below. Let me know if I need anything else (test cases, etc).

https://github.com/dojo/dojox/pull/199

comment:8 Changed 4 years ago by dylans <dylan@…>

Resolution: fixed
Status: assignedclosed

In 7c88fa3a6a7e75003ac876bd14350ef3270f0752/dojox:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 
Note: See TracTickets for help on using tickets.