Reposting to bring this discussion back to life. We started having it on IRC.

On 09/28/2011 08:38 PM, Adam Young wrote:
So I decided to try to get an IP Address widget working. See the attached patch. It was fairly trivial.

However, this widget is not really all that useful by itself. It would need to work as a part of a multivalued_text widget in order to replace the widget used on the dnsrecord page. And looking at the multivalued text widget, I think you will agree that is going to be tricky.

The problem is that we don't really have the API set up for nesting. This has burnt us on the Sections as well as making more complex widgets. Usually, instead of reusing the widgets we have, it is easier to go straight to the HTML.

I think the crux comes from the 1-1 mapping between widgets and fields of the request/result JSON. For widget it is: load(record) and the load command knows how to find the value it needs inside the record. For save, on the other hand, it just returns the values it needs.


In order to make the widget scheme more nestable, the section section.load and save can do more work, such as scoping down the piece of the request record to just that portion required by the widget. Bascially, it can do what widget.load does, just externally


        var value = record[that.name];
        if (value instanceof Array) {
            widget.values = value;
        } else {
            widget.values = value ? [value] : [];
        }

        widget.writable = true;

        if (widget.param_info) { Re:
            if (widget.param_info.primary_key) {
                widget.writable = false;
            }

if (widget.param_info.flags && 'no_update' in that.param_info.flags) {
                widget.writable = false;
            }
        }
Re:
        if (widget.record.attributelevelrights) {
            var rights = that.record.attributelevelrights[that.name];
            if (!rights || rights.indexOf('w') < 0) {
                widget.writable = false;
            }
        }


This seems to break encapsulation, but I think it might be the right level of abstraction. We can make this code reusable by other sections by calling it something like section.widget_load.

load then becomes nothing more than a call to reset for most widgets. But, for the widgets that need nesting, or that need additional rendering logic (like select) we still have a place for them to over ride it. I think it is telling that most of the overridden load functions still cal the parent function first. I thik that is an indicator that what we have can be refactored.


Looking through widget.js, I'd say that most of the widgets can be safely redone this way. Text_area.load looks suspect, it should probbly be using the load functionality from the base class. table_widget will need some thought, but I think it will work is the value is set as an array, and then the code is called.

      if (that.values) {
            for (var i=0; i<that.values.length; i++) {
                var record = that.get_record(result, i);
                that.add_record(record);
            }
        }
        that.unselect_all();


I think that the save functions be OK as is. the multivalue save should be able to work with the result of calling sae from each individual text area.



So where would this help us. DNS records is probably the big one. With users, we might have to store multiple keys or certificates. We don't currently validate email addresses, and we could there. It would probably simplify the code for ACIs, but there really is no reason to put effort in to reworking that for its own sake.



The other possibility is that we could redo the multivalue widget as a section. I know we've done that else where. On the user page, the Email addresses and phone numbers would then each go into their own section....or we leave the multivlue widget there, but apply this approach else where. For DNS records, this means that each record type would basically become its own section. I don't favor this approach, but I could buy the argument that redoing the widget API is too much work.






_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to