Opened 4 years ago

Closed 4 years ago

#18592 closed enhancement (worksforme)

HasDropDown eats up information, wether popup was cancelled

Reported by: wpausch Owned by: bill
Priority: undecided Milestone: tbd
Component: Dijit Version: 1.10.4
Keywords: Cc:
Blocked By: Blocking:

Description

HasDropDown?.openDropDown contains the code snippet

var retVal = popup.open({
(...)
onExecute: function(){
    self.closeDropDown(true);
},
onCancel: function(){
    self.closeDropDown(true);
},

In my application, I have a custom datepicker popup based on _HasDropDown. However, I need to ignore the chosen Date if the user closed the popup using the ESCAPE key.

Unfortunately, the above code skippet eats up the information, why the popup closes. Both in the execute, and in the cancel case, just closeDropDown(true) is called.

My quick-and-dirty-solution to make my application work is, copying the code of closeDropDown completely into the subclass, and just in the cancel case calling closeDropDown(true, true).

Thus, I would like to argue for adding some second parameter to closeDropDown which answers the question wether we are in the Execute or in the Cancel case.

Change History (7)

comment:1 Changed 4 years ago by bill

The thing I don't understand is, why are you processing the chosen date in closeDropDown() at all? I think DateTextBox? handles it in a different place... but I've forgotten exactly how, so I'll need to trace through the code.

comment:2 Changed 4 years ago by ben hockey

i believe it works this way...

this.dropDown should have onCancel and onExecute methods and those are what trigger the snippets you have above. since you have access to this.dropDown yourself then in startup of your widget, you add listeners to onCancel and onExecute yourself - nothing is lost.

comment:3 Changed 4 years ago by wpausch

Bill: I´ve just read a bit into _DateTimeTextBox.js, and as far as I get it, the approach there is:

  • we are the textbox: "textBox = this", i.e. the DateTimeTextBox? controls dropdown and textbox in one class
  • this.dropDown is constructed as "new PopupProto?" in openDropDown, and injects the chosen Date into textBox in a passed onChange handler.

(correct me if I´m wrong here, I just looked into the code but never actually used that class in my code)

Why did I use closeDropDown? At first, I regarded overwriting closeDropDown if I want to do some work to be performed on closing the dropdown rather straightforward. Furthermore, in openDropDown, I just set this.dropDown to the standard dijit/Calendar:

this.calendar = new Calendar({
    value : jsDate
});
this.dropDown = this.calendar;
    		
return this.superOpenDropDown(); // The code I copied from _HasDropDown as stated above

... in closeDropDown I then fetch the chosen Date from the calendar, and do what I want with it:

(FancyDate? and DateTimeSpinBox? are custom classes we wrote for our projects).

        closeDropDown: function(focus, aborted) {
        	// closeDropDown is called on multiple occurrences, for example on blur, even if the drop down is already closed.
        	// (Example: User chooses a date in the dropdown, closes the dropdown, then switches to another browser instance.
        	//  In that moment, a blur event will be emitted, which causes this code to be called.)
        	if (this.calendar && !this._disabled && !aborted) {
            	var jsDate = this.calendar.get("value");
            	var fancyDate = this.get("value").value;
            	if (fancyDate == null) {
            		fancyDate = new FancyDate({
            			     day : jsDate.getDate(),
            			   month : jsDate.getMonth() + 1,
            			    year : jsDate.getFullYear(),
            			timeZone : this.timeZone
            		});
            	} else {
                	fancyDate.setDay(jsDate.getDate());
                	fancyDate.setMonth(jsDate.getMonth() + 1);
                	fancyDate.setYear(jsDate.getFullYear());            		
            	}            	

            	this.dateTimeSpinBox.set("value", fancyDate);
        	}
        	
            this.inherited(arguments);
            
            this.emitValueChangeTopic();

            delete this.calendar; // I know, I could make them one variable...
            delete this.dropDown;
        },

I´m not completely sure wether I could have done things like in _DateTimeTextBox, but at least I regard that code quite modular. Also, how does the onChange handler in _DateTimeTextBox keep track of the case that the date isn´t to be chosen because the user hit Escape --- or isn´t this possible due to the nature of that widget?

Also, some details like emitting a topic are the result of rather hard work to make things work in the context of a dgrid --- I use that widget in various cells of a dgrid.

neonstalwart: I see this code in popup.open:

// watch for cancel/execute events on the popup and notify the caller
// (for a menu, "execute" means clicking an item)
if(widget.onCancel && args.onCancel){
    handlers.push(widget.on("cancel", args.onCancel));
}

Do you mean this one?

comment:4 Changed 4 years ago by ben hockey

yeah, widget is the popup widget, which in your textbox will be this.dropDown. closeDropDown is called in response to 2 signals that you should have access to already (this.dropDown.onExecute and this.dropDown.onCancel). so rather than getting those signals indirectly (via closeDropDown) with less information than you need, go back to the source of the signals (this.dropDown) and add your own listeners.

comment:5 Changed 4 years ago by bill

One thing to add: Complex popups have an onExecute() and onCancel() methods (like @neonstalwart) mentioned, but simple popups like dijit/Calendar just have an onChange() method.

Note that onChange() is only called when the user selects a value, not when he closes the dropdown via the ESC key. In other words, this question doesn't really make sense:

how does the onChange handler in _DateTimeTextBox keep track of the case that the date isn´t to be chosen because the user hit Escape ...?

... because the onChange handler isn't even called in that case.

So, although this enhancement would be trivial, I don't really see a reason for it. Let us know if there's some reason you can't just use the popup's onChange or onExecute method directly; otherwise I'll close the ticket.

Last edited 4 years ago by bill (previous) (diff)

comment:6 Changed 4 years ago by wpausch

Thank you for your help, the onChange handler does the job.

Then this ticket can indeed be closed.

comment:7 Changed 4 years ago by bill

Resolution: worksforme
Status: newclosed

OK, great!

Note: See TracTickets for help on using tickets.