PDA

View Full Version : Custom Widget implementing Field properties, can it be done cleaner?



Munter
12 Jan 2010, 7:24 AM
Recently I needed to create a custom widget to present some data from a Record in another way than any predefined Field could. To put together this new custom widget I had to incude some other widgets as items and also needed to use a layout manager.

So my problem was that Field inherits from BoxComponent, which doesn't have any layout manager and doesn't allow items. I needed Container to use my other widgets as items. which also inherits from Boxcomponent. This makes it impossible to set a value on my widget based on the loaded Record on the parenting FormPanel.

So what I did was a huge hack. I implemented the minimum methods and properties that BasicForm requires of a Field in order to make BasicForm think that my widget was actually a Field. This enabled me to set and get data from my widget using Records.

This is the ugly hack I used:



one.FakeField = Ext.extend(Ext.Container, {
// This ensures that BasicForm can load Record from Store using BasicForm.loadRecord()
isFormField: true,

// This ensures that BasicForm can load Record from Store using BasicForm.loadRecord()
getName: function () {
return this.name;
},

// This ensures that BasicForm can load Record from Store using BasicForm.loadRecord()
markInvalid: Ext.emptyFn,

// This ensures that BasicForm can load Record from Store using BasicForm.loadRecord()
clearInvalid: Ext.emptyFn,

// This ensures that FormPanel can "validate" FakeField
isValid: function () {
return true;
},

// This should be overridden by the subclass
setValue: function (value) {
this.value = value;
},

// This should be overridden by the subclass
getValue: function () {
return this.value;
}
});


So now to my question. Isn't there a better way to have Field functionality and be able to use a layout manager with my custom widgets?

--
Munter

Animal
12 Jan 2010, 7:31 AM
That looks fine, and not a hack. You needed a Container, so that's what you used. And if it has to be managed as a form field, you have to do a bit of "Multiple inheritance", and make it look like a Field too. Good solution.

Munter
12 Jan 2010, 7:52 AM
Wouldn't this be one of the cases where it would be beneficial for developers to forget a bit about the class hirarchy and make BasicForm a bit more flexible?

I mean, why should it be needed to implement much more then thre methods and a property.

It makes perfect sense that anyone who wants to implement the Field interface would have to implement setValue, getValue and getName. But apart from that, wouldn't it make sense to only need one other thing to make BasicForm accept the component as a Field. the 'isFormField' boolean should be enough.

However, right now BasicForm checks for markInvalid and clearInvalid methods, and it calls isValid without even checking if it's there. It would make a lot of sense to change BasicForms implementation so it would accept any item with isFormField set to true as a Field that implements getName, setValue and getValue, and only runs optional extra methods on said component if they are present.

Does any of this make sense?

I can try and see if I can come up with a patch. Hovever I only have a single test case, so I might not spot all the methods that BasicForm assumes are present on Field.

--
Munter

Animal
12 Jan 2010, 7:59 AM
Yes, makes sense.

Munter
13 Jan 2010, 7:45 AM
I have looked a bit at it now, and this is what I have come up with.

Any Component that wants to act like a Field, enabling it to interacts with BasicForm s loading of Records, the only interface it has to satisfy is the following:



FakeField = Ext.extend(Ext.Container, {
// This ensures that BasicForm can load Record from Store using BasicForm.loadRecord()
isFormField: true,

// This ensures that BasicForm can load Record from Store using BasicForm.loadRecord()
getName: function() { return this.name; },

// This should be overridden by the subclass
setValue : function(value){
this.value = value;
},

// This should be overridden by the subclass
getValue : function() {
return this.value;
}
});
All other methods that Field implements for interacting with BasicForm or FormPanel are optional. Though there are some assumptions:


The value of the field is always valid if the methods for checking and updating validity aren't implemented.
The field is always dirty if the methods for checking are not present.


These are the changes I have made to make it work.



// Proposal to change BasicForm and FormPanel to treat custom components as Fields if they ask to.
// http://www.extjs.com/forum/showthread.php?p=426559
Ext.override(Ext.form.BasicForm, {
isValid: function () {
var valid = true;
this.items.each(function (f) {
if (typeof f.validate === 'function' && !f.validate()) {
valid = false;
}
});
return valid;
},

isDirty: function () {
var dirty = false;
this.items.each(function (f) {
if (typeof f.isDirty !== 'function' || (typeof f.isDirty !== 'function' && f.isDirty())) {
dirty = true;
return false;
}
});
return dirty;
},

getFieldValues: function (dirtyOnly) {
var o = {},
n, key, val;
this.items.each(function (f) {
if (dirtyOnly !== true || typeof f.isDirty !== 'function' || (typeof f.isDirty === 'function' && f.isDirty())) {
n = f.getName();
key = o[n];
val = f.getValue();

if (Ext.isDefined(key)) {
if (Ext.isArray(key)) {
o[n].push(val);
} else {
o[n] = [key, val];
}
} else {
o[n] = val;
}
}
});
return o;
},

markInvalid: function (errors) {
if (Ext.isArray(errors)) {
for (var i = 0, len = errors.length; i < len; i++) {
var fieldError = errors[i];
var f = this.findField(fieldError.id);
if (f && typeof f.markInvalid === 'function') {
f.markInvalid(fieldError.msg);
}
}
} else {
var field, id;
for (id in errors) {
if (!Ext.isFunction(errors[id]) && (field = this.findField(id))) {
if (typeof field.markInvalid === 'function') {
field.markInvalid(errors[id]);
}
}
}
}
return this;
},

clearInvalid: function () {
this.items.each(function (f) {
if (typeof f.clearInvalid === 'function') {
f.clearInvalid();
}
});
return this;
},

reset: function () {
this.items.each(function (f) {
if (typeof f.reset === 'function') {
f.reset();
}
});
return this;
}
});

Ext.override(Ext.form.FormPanel, {
isField: function (c) {
return c.isFormField && c.getName && c.getValue && c.setValue;
},

bindHandler: function () {
var valid = true;
this.form.items.each(function (f) {
if (typeof f.isValid === 'function' && !f.isValid(true)) {
valid = false;
return false;
}
});
if (this.fbar) {
var fitems = this.fbar.items.items;
for (var i = 0, len = fitems.length; i < len; i++) {
var btn = fitems[i];
if (btn.formBind === true && btn.disabled === valid) {
btn.setDisabled(!valid);
}
}
}
this.fireEvent('clientvalidation', this, valid);
}
});
I haven't used all corners of BasicForm and FormPanel yet. Is there something I am missing, and am I on the right track with this?

--
Munter

Animal
13 Jan 2010, 7:48 AM
Looks OK, apart from, a "field" without an isDirty method always counts as dirty. Is that what you want?

Also, I'd use Ext.isFunction()

Munter
13 Jan 2010, 8:34 AM
Looks OK, apart from, a "field" without an isDirty method always counts as dirty. Is that what you want?

I figured that if there is no way to check wether or not a field is valid or dirty, it would be better to always assume valid and dirty. The reason for this is that a form may be set up to not save the input if no field is dirty. So of course this might be annoying to the people who try to make this optimization. But on the other hand those people probably know how to read the source and can see that if they implement isDirty themselves, the problem is solved.

So it is an assumption that benefits the developer who just want a quick fix, which I would assume are most developers out there. For the rest, implementing isDirty isn't that hard.



Also, I'd use Ext.isFunction()

I missed that one :)

I have updated the proposal and also trid to highlight it a bit better to spot the changes more easily. Also I have updated the isField check in FormPanel to only check the FakeField.isFormField boolean. I would assume that people notice FormPanel throwing an error right away when it is trying to call getvalue, setValue og getName when they aren't there.



// Proposal to change BasicForm and FormPanel to treat custom components as Fields if they ask to.
// http://www.extjs.com/forum/showthread.php?p=426559

Ext.override(Ext.form.BasicForm, {
isValid : function(){
var valid = true;
this.items.each(function(f){
if(Ext.isFunction(f.validate) && !f.validate()){
valid = false;
}
});
return valid;
},

isDirty : function(){
var dirty = false;
this.items.each(function(f){
if(!Ext.isFunction(f.isDirty) || (Ext.isFunction(f.isDirty) && f.isDirty())){
dirty = true;
return false;
}
});
return dirty;
},

getFieldValues : function(dirtyOnly){
var o = {},
n,
key,
val;
this.items.each(function(f){
if(dirtyOnly !== true || !Ext.isFunction(f.isDirty) || (Ext.isFunction(f.isDirty) && f.isDirty())){
n = f.getName();
key = o[n];
val = f.getValue();

if(Ext.isDefined(key)){
if(Ext.isArray(key)){
o[n].push(val);
}else{
o[n] = [key, val];
}
}else{
o[n] = val;
}
}
});
return o;
},

markInvalid : function(errors){
if(Ext.isArray(errors)){
for(var i = 0, len = errors.length; i < len; i++){
var fieldError = errors[i];
var f = this.findField(fieldError.id);
if(f && Ext.isFunction(f.markInvalid)){
f.markInvalid(fieldError.msg);
}
}
}else{
var field, id;
for(id in errors){
if(!Ext.isFunction(errors[id]) && (field = this.findField(id))){
if (Ext.isFunction(field.markInvalid)) {
field.markInvalid(errors[id]);
}
}
}
}
return this;
},

clearInvalid : function(){
this.items.each(function(f){
if (Ext.isFunction(f.clearInvalid)) {
f.clearInvalid();
}
});
return this;
},

reset : function(){
this.items.each(function(f){
if (Ext.isFunction(f.reset)) {
f.reset();
}
});
return this;
}
});

Ext.override(Ext.form.FormPanel, {
isField : function(c) {
return c.isFormField /* !!c.setValue && !!c.getValue && !!c.markInvalid && !!c.clearInvalid */;
},

bindHandler : function(){
var valid = true;
this.form.items.each(function(f){
if(Ext.isFunction(f.isValid) && !f.isValid(true)){
valid = false;
return false;
}
});
if(this.fbar){
var fitems = this.fbar.items.items;
for(var i = 0, len = fitems.length; i < len; i++){
var btn = fitems[i];
if(btn.formBind === true && btn.disabled === valid){
btn.setDisabled(!valid);
}
}
}
this.fireEvent('clientvalidation', this, valid);
}
});

Munter
14 Jan 2010, 4:16 AM
Since I have been working a bit with the Slider widget previously, I realised that it is quite simple to modify it to satisfy the interface for Field given the previous proposal.

This is the changeset:



Ext.override(Ext.Slider, {
isFormField : true,

getName : function() {
return this.name
}
});
Presto. Slider is now mimicking Field and can thus be used on a BasicForm that loads Records.

Animal
14 Jan 2010, 4:44 AM
It doesn't get submitted though does it?

Munter
14 Jan 2010, 6:09 AM
It doesn't get submitted though does it?

I don't know, I haven't been using FormPanel to do any actual POST request since I am working on a RIA where another component is responsible for talking to the backend. I have primarily focused on getting Stores and Records to work with Forms with as little coding as possible.

I would think that since Slider doesn't keep an input element in the dom, the html Form element won't submit that input either.

Now, I can't really see why people would really want to use standardsubmit when they have much more power available, but if it is supported in Ext, of course any component that acts like a field should do some housekeeping with an input element in the form.

for Slider the update would look like this:



Ext.override(Ext.Slider, {
isFormField : true,

getName : function() {
return this.name;
},


afterRender : function(){
Ext.Slider.superclass.afterRender.apply(this, arguments);
if (this.isFormField) {
this.shadowInput = Ext.DomHelper.append(this.el, {
tag: 'input',
type: 'hidden',
name: this.getName()
});
}
if(this.value !== undefined){
var v = this.normalizeValue(this.value);
if(v !== this.value){
delete this.value;
this.setValue(v, false);
}else{
this.moveThumb(this.translateValue(v), false);
}
}
},

setValue : function(v, animate, changeComplete){
v = this.normalizeValue(v);
if(v !== this.value && this.fireEvent('beforechange', this, v, this.value) !== false){
this.value = v;
if (this.shadowInput) {
this.shadowInput.value = v;
}
this.moveThumb(this.translateValue(v), animate !== false);
this.fireEvent('change', this, v);
if(changeComplete){
this.fireEvent('changecomplete', this, v);
}
}
}
});


This is untested since I can't get standardSubmit to work right now on my test setup. But the input is correctly updated.

I am here assuming that anyone who creates a widget that follows the Field interface will also be responsible for creating a housekeeping input element if said widget is supposed to be compatible with standard form submissions.

I couldn't find any hook in BasicForm or FormPanel to take away that responsibility from the widget developer. Though there might be a possibility of changing the behavior of standardSubmit to actually still use the XHR data gathering method and create the missing input elements itself.

Animal
14 Jan 2010, 6:37 AM
That looks good.

IMNSHO, Slider should always have been a form Field and implemented the usual Field interface.

It's a way of gathering user input. It's just bad engineering the way it is now.

I've had to implement a SliderField class in a project recently.