Dialog: Enhanced Button Option

Consider allowing an id to be specified for each button (see #4047).

Replying to [comment:1 scott.gonzalez]:

Consider allowing an id to be specified for each button (see #4047).

Yeah Id would be nice.

I hope it doesn't concern the jq ui devs that 1.7 is out with buttons not ordered in the same way they were added because fixing it will make it a backwards incompatible change.

Anyway is there someone working on this?

I added a svn diff for this. Not sure if thats the way to contribute with patches.

Anyway. The diff makes the following possible:

$('#foo').dialog({

'buttons' : {

'primary' : {

id:'fooButton',

priority: 'primary',

click: function() {

...

},

},

'secondary' : {

click: function() {

...

},

priority: 'secondary'

},

'disabled' : {

disabled: true

}

}

});

Im kind of a beginner at unit tests, I have checked the tests for dialog and I think I can manage to make tests for these changes BUT I wan't someone to review the patch to check if it's OK first and I also like the following to be discussed:

• I realize the click callback is now optional. Is that OK?

• If no id is set, the default '' will be used and the DOM will look like <button id="" ...>..</button> Is that OK? I think it's ugly but doing $('<button></button>').attr('id', options.id).prependTo(..) is so much nicer than var $button = $('<button></button>'); if(options.id !== '') { $button.attr('id', options.id) } $button.prependTo(..)

Differences between ui.dialog.diff and ui.dialog.diff.2

The first patch makes it possible to add custom css classes

$('#foo').dialog({

'buttons' : {

'cancel' : {

priority: 'secondary',

classes: 'foo bar baz',

click: function() {

...

},

},

}

});

Adding custom classes skips the ui-corner-all class. Example of usage: someone want's to make a cancel button look like a link instead of a rounded corner button. Though secondary priority and custom classes could be combined. Another example of usage is if someone would like only some of the corners rounded. The ui-corner-all class could be added with custom classes if needed.

The second patch takes another approach. Instead of allowing custom classes it takes advantage of the fact that you can put any string into priority. This applies to first patch aswell though.

Example:

$('#foo').dialog({

'buttons' : {

'foobar' : {

priority: 'custom-prio',

click: function() {

...

},

},

}

});

The button will then get a class of 'ui-priority-custom-prio'. To disable rounded corners with this patch one could use cornerAll: false

$('#foo').dialog({

'buttons' : {

'foobar' : {

priority: 'custom-prio',

cornerAll: false,

click: function() {

...

},

},

}

});

Im not sure either of this making it possible to customize the buttons is something wanted. It's probably very rare use-cases. I mean, either you use the jquery ui css framework or don't, not half of it.

Please discuss this and ideas and let me now how I can improve the patch.

Link to new discussion thread: http://groups.google.com/group/jquery-ui-dev/browse_thread/thread/1e16804e5753bbc4#

See #5287 for a proposed patch (not reviewed).

http://github.com/azatoth/jquery-ui/commit/5fd59d834fe73f178de7d528a09c297a47cfb9e1

The current buttons implementation is extremely simple by design as it's a convenience. What about leaving it as is, and if you specify buttons: true you get the button pane, to which you can add your own buttons manually. I don't really see any end to the amount of access one might need to these dom elements (hover, focus, calling .button() on them, changing icons, hiding/showing, changing text, etc), and going through a dialog.buttons API to do all this just difficults things. It's not really a concern of the dialog how many buttons there are, in which order they're displayed, which is primary, etc. so it shouldn't get involved beyond creating a container for them.

Replying to [comment:11 rdworth]:

The current buttons implementation is extremely simple by design as it's a convenience. What about leaving it as is, and if you specify buttons: true you get the button pane, to which you can add your own buttons manually. I don't really see any end to the amount of access one might need to these dom elements (hover, focus, calling .button() on them, changing icons, hiding/showing, changing text, etc), and going through a dialog.buttons API to do all this just difficults things. It's not really a concern of the dialog how many buttons there are, in which order they're displayed, which is primary, etc. so it shouldn't get involved beyond creating a container for them.

The main reason for the change is to allow people to actually be able to set an id on the button without having to create the buttons manually.

Personally I'm against using labels as keys, as labels has the habit of being non-static (translations etc), and a design that is meant to only work with the most simplistic system seems to me to be the wrong way to do it.

Replying to [comment:11 rdworth]:

The current buttons implementation is extremely simple by design as it's a convenience. What about leaving it as is, and if you specify buttons: true you get the button pane, to which you can add your own buttons manually. I don't really see any end to the amount of access one might need to these dom elements (hover, focus, calling .button() on them, changing icons, hiding/showing, changing text, etc), and going through a dialog.buttons API to do all this just difficults things. It's not really a concern of the dialog how many buttons there are, in which order they're displayed, which is primary, etc. so it shouldn't get involved beyond creating a container for them.

What does "you get the button pane" mean? Could you provide an example?

Replying to [comment:12 AzaToth]:

Replying to [comment:11 rdworth]: > The current buttons implementation is extremely simple by design as it's a convenience. What about leaving it as is, and if you specify buttons: true you get the button pane, to which you can add your own buttons manually. I don't really see any end to the amount of access one might need to these dom elements (hover, focus, calling .button() on them, changing icons, hiding/showing, changing text, etc), and going through a dialog.buttons API to do all this just difficults things. It's not really a concern of the dialog how many buttons there are, in which order they're displayed, which is primary, etc. so it shouldn't get involved beyond creating a container for them. The main reason for the change is to allow people to actually be able to set an id on the button without having to create the buttons manually. Personally I'm against using labels as keys, as labels has the habit of being non-static (translations etc), and a design that is meant to only work with the most simplistic system seems to me to be the wrong way to do it.

You're arguing that we should do away with the simple system and replace it with a complex one because it doesn't meet the complex needs. I'm saying we should keep the simple system and not build in the the complex one at all, but have enough hooks that it can be added on. This is similar to the design of the autocomplete - it does basic local data sources out of the box. If you want to do remote data sources and caching, you're on your own, but the hooks are there to do it right in the right places.

Replying to [comment:13 joern.zaefferer]:

Replying to [comment:11 rdworth]: > The current buttons implementation is extremely simple by design as it's a convenience. What about leaving it as is, and if you specify buttons: true you get the button pane, to which you can add your own buttons manually. I don't really see any end to the amount of access one might need to these dom elements (hover, focus, calling .button() on them, changing icons, hiding/showing, changing text, etc), and going through a dialog.buttons API to do all this just difficults things. It's not really a concern of the dialog how many buttons there are, in which order they're displayed, which is primary, etc. so it shouldn't get involved beyond creating a container for them. What does "you get the button pane" mean? Could you provide an example?

You would call

.dialog({ buttons: true })

and the dialog would generate the .ui-dialog-buttonpane element (as it does now if buttons is specified), to which you could manually append your manually created buttons.

.dialog("widget").find(".ui-dialog-buttonpane").append(...)

Replying to [comment:14 rdworth]:

You're arguing that we should do away with the simple system and replace it with a complex one because it doesn't meet the complex needs. I'm saying we should keep the simple system and not build in the the complex one at all, but have enough hooks that it can be added on. This is similar to the design of the autocomplete - it does basic local data sources out of the box. If you want to do remote data sources and caching, you're on your own, but the hooks are there to do it right in the right places.

The main issue here I say is that the current system makes it way to cumbersome if you just want to say add an class, use translated labels, add an image to an label etc... You shouldn't need to rewrite the logic of your code just to be able to do such things. And personally, I feel it's simpler to have static named labels, as it reduces the cognitive load to identify structures; True, I havn't don't any survey how it is out in the world regarding if current system, or my proposed system is best/easiest to use, so I have only my rather technical-wired mind to work with :).

I think a good first step to improving the API is to create a new button instance and extend the button widget's options for each button. The API might change sightly, but would give you immediate access to all current (and future) options, and then some. It wouldn't be too difficult to retain backwards compatibility.

An API like this makes sense to me:

buttons: {
	"OK": {
		disabled: true, // available by extending the button widget
		icons: {}, // available by extending the button widget
		priority: 1,
		id: "ok-button",
		handler: function(){
			// do something on click
		}
	}
}

If you really wanted to interact with the buttons after initialization, let the button widget do the heavy lifting.

$("#button-ok").button("disable");

// or if you must, some kind of getter
$("#dialog").dialog("buttons", "button-ok").button("disable");

Allowing people to add and remove buttons after initialization seems like an edge case to me. Create all the buttons and either disable/enable/hide/show them based on actions taken inside the dialog.

I've got a pretty simple patch that adds as much flexibility as you want, but requires jQuery 1.4.

Diff: http://gist.github.com/554334

Demo: http://jsbin.com/aroje/edit

Replying to [comment:18 scott.gonzalez]:

I've got a pretty simple patch that adds as much flexibility as you want, but requires jQuery 1.4. Diff: http://gist.github.com/554334 Demo: http://jsbin.com/aroje/edit

/me likes

(didn't know .each() worked both for Array and for Object)

Fixed in 95a3459.

Dialog: Added additional syntax for creating buttons. Fixes #4344 - Dialog: Enhanced Button Option.

Changeset: 95a34593f98217e3a99f223e2ef3ca0a0fa8343b