On 10/21/2011 6:40 AM, Petr Vobornik wrote:
1) Wouldn't be better if the asterisk has different color than the
label? Visually I don't like it that much and I think it can be
overlook. Attaching a proposition. I used green IPAish color because red
usually means error.

Code from browser how it was done:

<td class="section-cell-label" title="First name"><label
name="givenname" class="field-label">First name:</label><span
class="required" style="
float: right;
font-weight: bold;
color: #319016;
font-size: 20px;
" title="required">*</span></td>

(style should be moved to css file)


<div style="line-height: 25px;"><span class="required" style="
font-weight: bold;
color: #319016;
font-size: 20px;
vertical-align: middle;
">*</span> required</div>

It may vary on the section type.

Fixed. I couldn't use the exact style above because it looks a bit weird in the details page since the labels are right aligned. I use red for now because it can also mean 'important', but we can adjust this again later.

2) When rendering label, we should also obtain field input's id (if
possible) for 'for' attribute of <label>. This can be done separately.

I'd rather avoid using ID in a dynamic app like this. Adding ID into the fields is possible, but we'd have to do it very carefully to avoid conflicts.

3) Should we create some common pure html widgets with certain
semantics?

We can, but as discussed elsewhere, we have to fix the current assumption first: each widget correspond to an attribute, it currently has properties and methods (e.g. metadata, load(), save()) that are only valid on attributes.

IE asterisk shouldn't be directly concatenated with label
text. It is used on more than one place which may cause maintenance issues.

IPA.form(or some other name).required_indicator = function() {
return '*'
};

in this case this seems unnecessary. But if the required indicator was
like in 1) it will be useful.

Fixed.

Summary:
All 3 points are nice to have. If you think is not necessary then ACK.

This patch is also fixing https://fedorahosted.org/freeipa/ticket/1973 .

Noted in the new patch.

--
Endi S. Dewata
From 3d19fd7ab618017e144b848b38a7e1bd7da95d6d Mon Sep 17 00:00:00 2001
From: Endi S. Dewata <edew...@redhat.com>
Date: Wed, 19 Oct 2011 18:11:09 -0200
Subject: [PATCH] Fixed inconsistent required/optional attributes.

The dialogs and details pages have been modified to use the * symbol
to mark required fields. The automount map and the DNS zone dialogs
have been modified to update the required fields according to the
input type.

Ticket #1696, #1973
---
 install/ui/aci.js                  |    6 +-
 install/ui/add.js                  |  144 ++++++++++++++----------
 install/ui/automount.js            |   24 +++--
 install/ui/details.js              |   27 +----
 install/ui/dns.js                  |  216 +++++++++++++++---------------------
 install/ui/hbac.js                 |    3 +-
 install/ui/host.js                 |   18 +++-
 install/ui/ipa.css                 |    7 +
 install/ui/policy.js               |    8 +-
 install/ui/service.js              |   63 ++++++-----
 install/ui/sudo.js                 |    3 +-
 install/ui/test/data/ipa_init.json |    1 -
 install/ui/user.js                 |    5 +-
 install/ui/widget.js               |   56 ++++++++--
 ipalib/plugins/internal.py         |    1 -
 15 files changed, 311 insertions(+), 271 deletions(-)

diff --git a/install/ui/aci.js b/install/ui/aci.js
index 8dcb540b6a030222136e95d5d01b5a11c29acd7f..7a331118afb05df720429f004202b410ed29eac5 100644
--- a/install/ui/aci.js
+++ b/install/ui/aci.js
@@ -35,11 +35,7 @@ IPA.entity_factories.permission = function() {
             {
                 name: 'identity',
                 fields: [
-                    {
-                        factory: IPA.text_widget,
-                        name: 'cn',
-                        read_only: true
-                    }
+                    'cn'
                 ]
             },
             {
diff --git a/install/ui/add.js b/install/ui/add.js
index 17418aaba4f0fe623483323a13c3d5cd608f4c71..fd99b02c5eb77acc99484995a6fb0c65598128b7 100644
--- a/install/ui/add.js
+++ b/install/ui/add.js
@@ -35,7 +35,72 @@ IPA.add_dialog = function (spec) {
     that.retry = typeof spec.retry !== 'undefined' ? spec.retry : true;
     that.command = null;
 
-    function show_edit_page(entity,result){
+    that.show_edit_page = spec.show_edit_page || show_edit_page;
+
+    var init = function() {
+        that.create_button({
+            name: 'add',
+            label: IPA.messages.buttons.add,
+            click: function() {
+                that.hide_message();
+                that.add(
+                    function(data, text_status, xhr) {
+                        var facet = IPA.current_entity.get_facet();
+                        var table = facet.table;
+                        table.refresh();
+                        that.close();
+                    },
+                    that.on_error);
+            }
+        });
+
+        that.create_button({
+            name: 'add_and_add_another',
+            label: IPA.messages.buttons.add_and_add_another,
+            click: function() {
+                that.hide_message();
+                that.add(
+                    function(data, text_status, xhr) {
+                        var label = that.entity.metadata.label_singular;
+                        var message = IPA.messages.dialogs.add_confirmation;
+                        message = message.replace('${entity}', label);
+                        that.show_message(message);
+
+                        var facet = IPA.current_entity.get_facet();
+                        var table = facet.table;
+                        table.refresh();
+                        that.reset();
+                    },
+                    that.on_error);
+            }
+        });
+
+        that.create_button({
+            name: 'add_and_edit',
+            label: IPA.messages.buttons.add_and_edit,
+            click: function() {
+                that.hide_message();
+                that.add(
+                    function(data, text_status, xhr) {
+                        that.close();
+                        var result = data.result.result;
+                        that.show_edit_page(that.entity, result);
+                    },
+                    that.on_error);
+            }
+        });
+
+        that.create_button({
+            name: 'cancel',
+            label: IPA.messages.buttons.cancel,
+            click: function() {
+                that.hide_message();
+                that.close();
+            }
+        });
+    };
+
+    function show_edit_page(entity,result) {
         var pkey_name = entity.metadata.primary_key;
         var pkey = result[pkey_name];
         if (pkey instanceof Array) {
@@ -44,8 +109,6 @@ IPA.add_dialog = function (spec) {
         IPA.nav.show_entity_page(that.entity, 'default', pkey);
     }
 
-    that.show_edit_page = spec.show_edit_page || show_edit_page;
-
     that.add = function(on_success, on_error) {
 
         var pkey_name = that.entity.metadata.primary_key;
@@ -110,67 +173,28 @@ IPA.add_dialog = function (spec) {
         command.execute();
     };
 
-    /*dialog initialization*/
-    that.create_button({
-        name: 'add',
-        label: IPA.messages.buttons.add,
-        click: function() {
-            that.hide_message();
-            that.add(
-                function(data, text_status, xhr) {
-                    var facet = IPA.current_entity.get_facet();
-                    var table = facet.table;
-                    table.refresh();
-                    that.close();
-                },
-                that.on_error);
-        }
-    });
+    that.create = function() {
+        that.dialog_create();
 
-    that.create_button({
-        name: 'add_and_add_another',
-        label: IPA.messages.buttons.add_and_add_another,
-        click: function() {
-            that.hide_message();
-            that.add(
-                function(data, text_status, xhr) {
-                    var label = that.entity.metadata.label_singular;
-                    var message = IPA.messages.dialogs.add_confirmation;
-                    message = message.replace('${entity}', label);
-                    that.show_message(message);
+        var div = $('<div/>', {
+        }).appendTo(that.container);
 
-                    var facet = IPA.current_entity.get_facet();
-                    var table = facet.table;
-                    table.refresh();
-                    that.reset();
-                },
-                that.on_error);
-        }
-    });
+        $('<span/>', {
+            'class': 'required-indicator',
+            text: IPA.required_indicator
+        }).appendTo(div);
 
-    that.create_button({
-        name: 'add_and_edit',
-        label: IPA.messages.buttons.add_and_edit,
-        click: function() {
-            that.hide_message();
-            that.add(
-                function(data, text_status, xhr) {
-                    that.close();
-                    var result = data.result.result;
-                    that.show_edit_page(that.entity, result);
-                },
-                that.on_error);
-        }
-    });
+        div.append(' ');
 
-    that.create_button({
-        name: 'cancel',
-        label: IPA.messages.buttons.cancel,
-        click: function() {
-            that.hide_message();
-            that.close();
-        }
-    });
+        $('<span/>', {
+            text: IPA.messages.widget.validation.required
+        }).appendTo(div);
+    };
+
+    // methods that should be invoked by subclasses
+    that.add_dialog_create = that.create;
+
+    init();
 
     return that;
 };
diff --git a/install/ui/automount.js b/install/ui/automount.js
index 89b0f6b7eff2988a5f2efcac2358323ee956f767..6b740d8e6d4abc1d0e402f5a1a3b79e8a4a77420 100644
--- a/install/ui/automount.js
+++ b/install/ui/automount.js
@@ -144,17 +144,17 @@ IPA.entity_factories.automountkey = function() {
         entity({ name: 'automountkey' }).
         containing_entity('automountmap').
         details_facet({
-            sections:[
+            sections: [
                 {
                     name:'identity',
                     label: IPA.messages.details.identity,
-                    fields:[
+                    fields: [
                         {
-                            factory: IPA.text_widget,
-                            read_only: true,
-                            name:   'automountkey'
+                            name: 'automountkey',
+                            read_only: true
                         },
-                        'automountinformation']
+                        'automountinformation'
+                    ]
                 }
             ],
             disable_breadcrumb: false,
@@ -224,20 +224,26 @@ IPA.automountmap_adder_dialog = function(spec) {
     var that = IPA.add_dialog(spec);
 
     that.create = function() {
-        that.dialog_create();
+        that.add_dialog_create();
 
         var method_field = that.get_field('method');
+        var indirect_section = that.get_section('indirect');
+        var key_field = that.get_field('key');
 
         var direct_input = $('input[value="add"]', method_field.container);
         direct_input.change(function() {
             that.method = 'add';
-            that.get_section('indirect').set_visible(false);
+
+            key_field.set_required(false);
+            indirect_section.set_visible(false);
         });
 
         var indirect_input = $('input[value="add_indirect"]', method_field.container);
         indirect_input.change(function() {
             that.method = 'add_indirect';
-            that.get_section('indirect').set_visible(true);
+
+            key_field.set_required(true);
+            indirect_section.set_visible(true);
         });
 
         direct_input.click();
diff --git a/install/ui/details.js b/install/ui/details.js
index 1e4a9eb5f1148cb109a89d0affd6aee3c4c6eefd..5c03de0a32aed46aaebd36facddceaf56a853004 100644
--- a/install/ui/details.js
+++ b/install/ui/details.js
@@ -234,44 +234,29 @@ IPA.details_table_section = function(spec) {
             var tr = $('<tr/>').appendTo(table);
 
             var td = $('<td/>', {
-                'class': 'section-cell-label'
+                'class': 'section-cell-label',
+                title: field.label
             }).appendTo(tr);
 
             $('<label/>', {
                 name: field.name,
-                title: field.label,
                 'class': 'field-label',
                 text: field.label+':'
             }).appendTo(td);
 
+            field.create_required(td);
+
             td = $('<td/>', {
-                'class': 'section-cell-field'
+                'class': 'section-cell-field',
+                title: field.label
             }).appendTo(tr);
 
             var field_container = $('<div/>', {
                 name: field.name,
-                title: field.label,
                 'class': 'field'
             }).appendTo(td);
 
             field.create(field_container);
-
-            if (field.optional) {
-                field_container.css('display', 'none');
-
-                var link = $('<a/>', {
-                    text: IPA.messages.widget.optional,
-                    href: ''
-                }).appendTo(td);
-
-                link.click(function(field_container, link) {
-                    return function() {
-                        field_container.css('display', 'inline');
-                        link.css('display', 'none');
-                        return false;
-                    };
-                }(field_container, link));
-            }
         }
     };
 
diff --git a/install/ui/dns.js b/install/ui/dns.js
index 60505d999d78b8d22331b2a01cb0543cc613f548..2b98f0dfec4c8f70770123930ea06081ff47633a 100644
--- a/install/ui/dns.js
+++ b/install/ui/dns.js
@@ -106,21 +106,32 @@ IPA.entity_factories.dnszone = function() {
         adder_dialog({
             factory: IPA.dnszone_adder_dialog,
             height: 300,
-            fields: [
+            sections: [
                 {
-                    name: 'idnsname',
-                    optional: true
+                    factory: IPA.dnszone_name_section,
+                    name: 'name',
+                    fields: [
+                        {
+                            name: 'idnsname',
+                            required: false
+                        },
+                        'name_from_ip'
+                    ]
                 },
-                'name_from_ip',
-                'idnssoamname',
                 {
-                    name: 'idnssoarname',
-                    optional: true
-                },
-                {
-                    factory: IPA.force_dnszone_add_checkbox_widget,
-                    name: 'force',
-                    param_info: IPA.get_method_option('dnszone_add', 'force')
+                    name: 'other',
+                    fields: [
+                        'idnssoamname',
+                        {
+                            name: 'idnssoarname',
+                            required: false
+                        },
+                        {
+                            factory: IPA.force_dnszone_add_checkbox_widget,
+                            name: 'force',
+                            param_info: IPA.get_method_option('dnszone_add', 'force')
+                        }
+                    ]
                 }
             ]
         }).
@@ -231,200 +242,157 @@ IPA.dnszone_details_facet = function(spec) {
     return that;
 };
 
-// TODO: Remove the custom create() by moving the fields into sections.
-// The idnsname and name_from_ip should be moved into a custom section.
-// The idnssoamname, idnssoarname, and force into a standard section.
-IPA.dnszone_adder_dialog = function(spec) {
+IPA.dnszone_name_section = function(spec) {
 
     spec = spec || {};
 
-    var that = IPA.add_dialog(spec);
+    var that = IPA.details_table_section(spec);
 
-    that.create = function() {
-
-        that.container.addClass('dnszone-adder-dialog');
+    that.create = function(container) {
+        that.container = container;
 
         that.message_container = $('<div/>', {
             style: 'display: none',
             'class': 'dialog-message ui-state-highlight ui-corner-all'
         }).appendTo(that.container);
 
-        var table = $('<table/>').appendTo(that.container);
+        var table = $('<table/>', {
+            'class': 'section-table'
+        }).appendTo(that.container);
 
-        var field = that.get_field('idnsname');
+        var idnsname = that.get_field('idnsname');
 
         var tr = $('<tr/>').appendTo(table);
 
         var td = $('<td/>', {
-            title: field.label
+            'class': 'section-cell-label',
+            title: idnsname.label
         }).appendTo(tr);
 
         var label = $('<label/>', {
+            name: 'idnsname',
+            'class': 'field-label',
             'for': 'dnszone-adder-dialog-idnsname-radio'
         }).appendTo(td);
 
-        that.idnsname_radio = $('<input/>', {
+        idnsname.radio = $('<input/>', {
             type: 'radio',
             id: 'dnszone-adder-dialog-idnsname-radio',
             name: 'type',
-            value: 'idnsname'
+            value: idnsname.name
         }).appendTo(label);
 
-        label.append(field.label+':');
+        label.append(idnsname.label+':');
+
+        idnsname.create_required(td);
 
         td = $('<td/>', {
-            title: field.label
+            'class': 'section-cell-field',
+            title: idnsname.label
         }).appendTo(tr);
 
         var span = $('<span/>', {
-            name: field.name
+            name: 'idnsname',
+            'class': 'field'
         }).appendTo(td);
 
-        field.create(span);
+        idnsname.create(span);
 
         var idnsname_input = $('input', span);
 
-        field = that.get_field('name_from_ip');
+        var name_from_ip = that.get_field('name_from_ip');
 
         tr = $('<tr/>').appendTo(table);
 
         td = $('<td/>', {
-            title: field.label
+            'class': 'section-cell-label',
+            title: name_from_ip.label
         }).appendTo(tr);
 
         label = $('<label/>', {
+            name: 'name_from_ip',
+            'class': 'field-label',
             'for': 'dnszone-adder-dialog-name_from_ip-radio'
         }).appendTo(td);
 
-        var name_from_ip_radio = $('<input/>', {
+        name_from_ip.radio = $('<input/>', {
             type: 'radio',
             id: 'dnszone-adder-dialog-name_from_ip-radio',
             name: 'type',
-            value: 'name_from_ip'
+            value: name_from_ip.name
         }).appendTo(label);
 
-        label.append(field.label+':');
+        label.append(name_from_ip.label+':');
+
+        name_from_ip.create_required(td);
 
         td = $('<td/>', {
-            title: field.label
+            'class': 'section-cell-field',
+            title: name_from_ip.label
         }).appendTo(tr);
 
         span = $('<span/>', {
-            name: field.name
+            name: 'name_from_ip',
+            'class': 'field'
         }).appendTo(td);
 
-        field.create(span);
+        name_from_ip.create(span);
 
         var name_from_ip_input = $('input', span);
 
-        that.idnsname_radio.click(function() {
+        idnsname.radio.click(function() {
             idnsname_input.attr('disabled', false);
             name_from_ip_input.attr('disabled', true);
+
+            idnsname.set_required(true);
+            name_from_ip.set_required(false);
+
+            name_from_ip.reset();
         });
 
-        name_from_ip_radio.click(function() {
+        name_from_ip.radio.click(function() {
             idnsname_input.attr('disabled', true);
             name_from_ip_input.attr('disabled', false);
-        });
 
-        idnsname_input.focus(function() {
-            that.idnsname_radio.attr('checked', true);
-        });
+            idnsname.set_required(false);
+            name_from_ip.set_required(true);
 
-        name_from_ip_input.focus(function() {
-            name_from_ip_radio.attr('checked', true);
+            idnsname.reset();
         });
 
-        that.idnsname_radio.click();
-
-        tr = $('<tr/>').appendTo(table);
-
-        td = $('<td/>', {
-            colspan: 2,
-            html: '&nbsp;'
-        }).appendTo(tr);
-
-        field = that.get_field('idnssoamname');
-
-        tr = $('<tr/>').appendTo(table);
-
-        td = $('<td/>', {
-            title: field.label
-        }).appendTo(tr);
-
-        label = $('<label/>', {
-            text: field.label+':'
-        }).appendTo(td);
-
-        td = $('<td/>', {
-            title: field.label
-        }).appendTo(tr);
-
-        span = $('<span/>', {
-            name: field.name
-        }).appendTo(td);
-
-        field.create(span);
-
-        field = that.get_field('idnssoarname');
-
-        tr = $('<tr/>').appendTo(table);
-
-        td = $('<td/>', {
-            title: field.label
-        }).appendTo(tr);
-
-        label = $('<label/>', {
-            text: field.label+':'
-        }).appendTo(td);
-
-        td = $('<td/>', {
-            title: field.label
-        }).appendTo(tr);
-
-        span = $('<span/>', {
-            name: field.name
-        }).appendTo(td);
-
-        field.create(span);
-
-        field = that.get_field('force');
-
-        tr = $('<tr/>').appendTo(table);
-
-        td = $('<td/>', {
-            title: field.label
-        }).appendTo(tr);
-
-        label = $('<label/>', {
-            text: field.label+':'
-        }).appendTo(td);
-
-        td = $('<td/>', {
-            title: field.label
-        }).appendTo(tr);
-
-        span = $('<span/>', {
-            name: field.name
-        }).appendTo(td);
-
-        field.create(span);
+        idnsname.radio.click();
     };
 
     that.save = function(record) {
 
-        that.dialog_save(record);
+        var idnsname = that.get_field('idnsname');
+        var name_from_ip = that.get_field('name_from_ip');
+
+        if (idnsname.radio.is(':checked')) {
+            record.idnsname = idnsname.save();
 
-        if (that.idnsname_radio.is(':checked')) {
-            delete record.name_from_ip;
         } else {
-            delete record.idnsname;
+            record.name_from_ip = name_from_ip.save();
         }
     };
 
     return that;
 };
 
+IPA.dnszone_adder_dialog = function(spec) {
+
+    spec = spec || {};
+
+    var that = IPA.add_dialog(spec);
+
+    that.create = function() {
+        that.add_dialog_create();
+        that.container.addClass('dnszone-adder-dialog');
+    };
+
+    return that;
+};
+
 IPA.dns_record_search_load = function (result) {
     this.table.empty();
     var normalized_record;
@@ -624,7 +592,7 @@ IPA.entity_factories.dnsrecord = function() {
                     name: 'record_data',
                     label: IPA.messages.objects.dnsrecord.data,
                     factory: IPA.text_widget,
-                    param_info: {required:true}
+                    required: true
                 }
             ]
         }).
diff --git a/install/ui/hbac.js b/install/ui/hbac.js
index c4b4877e2c2f5916ab3c926afa302f08e2927480..fb57dd1582c70aaf3c18eaee087751b1dd4c3b49 100644
--- a/install/ui/hbac.js
+++ b/install/ui/hbac.js
@@ -177,8 +177,7 @@ IPA.hbacrule_details_facet = function(spec) {
         });
 
         section.text({
-            name: 'cn',
-            read_only: true
+            name: 'cn'
         });
         section.textarea({
             name: 'description'
diff --git a/install/ui/host.js b/install/ui/host.js
index 552979b1a317c0a11254a10f3a77a5b70bf3d7a1..b50287291568c95830b275cdfa8dc3332677e2c2 100644
--- a/install/ui/host.js
+++ b/install/ui/host.js
@@ -124,14 +124,14 @@ IPA.entity_factories.host = function () {
                         {
                             factory: IPA.widget,
                             name: 'fqdn',
-                            optional: true,
+                            required: false,
                             hidden: true
                         },
                         {
                             factory: IPA.text_widget,
                             name: 'hostname',
                             label: IPA.messages.objects.service.host,
-                            param_info: { required: true }
+                            required: true
                         },
                         {
                             factory: IPA.dnszone_select_widget,
@@ -139,7 +139,7 @@ IPA.entity_factories.host = function () {
                             label: IPA.metadata.objects.dnszone.label_singular,
                             editable: true,
                             empty_option: false,
-                            param_info: { required: true }
+                            required: true
                         }
                     ]
                 },
@@ -190,12 +190,22 @@ IPA.host_fqdn_section = function(spec) {
             text: hostname.label
         }).appendTo(tr);
 
+        $('<span/>', {
+            'class': 'required-indicator',
+            text: IPA.required_indicator
+        }).appendTo(th);
+
         th = $('<th/>', {
             'class': 'dnszone',
             title: dnszone.label,
             text: dnszone.label
         }).appendTo(tr);
 
+        $('<span/>', {
+            'class': 'required-indicator',
+            text: IPA.required_indicator
+        }).appendTo(th);
+
         tr = $('<tr/>').appendTo(table);
 
         var td = $('<td/>', {
@@ -256,7 +266,7 @@ IPA.host_adder_dialog = function(spec) {
     var that = IPA.add_dialog(spec);
 
     that.create = function() {
-        that.dialog_create();
+        that.add_dialog_create();
         that.container.addClass('host-adder-dialog');
     };
 
diff --git a/install/ui/ipa.css b/install/ui/ipa.css
index 3c6cc6e820555e50b72c0970fe1c53859e3ff35f..be4ad361e6f9262a8ecb6a1744cce9cf25e29f1e 100644
--- a/install/ui/ipa.css
+++ b/install/ui/ipa.css
@@ -1256,6 +1256,10 @@ table.scrollable tbody {
     width: 100%;
 }
 
+.dnszone-adder-dialog .section-cell-label {
+    width: 180px;
+}
+
 /* Info and simple pages (not main app) */
 
 body.info-page  {
@@ -1294,3 +1298,6 @@ body.info-page  {
     padding: .15em;
 }
 
+.required-indicator {
+     color: red;
+}
\ No newline at end of file
diff --git a/install/ui/policy.js b/install/ui/policy.js
index 7fbf5600c38fcf285a9794342239a7af695beed0..54321a481634e900d68e58d5f8acb38ac6819b0a 100644
--- a/install/ui/policy.js
+++ b/install/ui/policy.js
@@ -52,14 +52,16 @@ IPA.entity_factories.pwpolicy = function() {
                 }]}).
         standard_association_facets().
         adder_dialog({
-            fields:[
+            fields: [
                 {
                     factory: IPA.entity_select_widget,
                     name: 'cn',
                     other_entity: 'group',
-                    other_field: 'cn'
+                    other_field: 'cn',
+                    required: true
                 },
-                'cospriority'],
+                'cospriority'
+            ],
             height: 300
         }).
         build();
diff --git a/install/ui/service.js b/install/ui/service.js
index 912642982ed24eb24c16c165ce5f308ba715a8d5..0ac2b6bec131786ca3f0c80122e9d8483a9c8f6c 100644
--- a/install/ui/service.js
+++ b/install/ui/service.js
@@ -30,39 +30,48 @@ IPA.entity_factories.service = function() {
         search_facet({
             columns: [ 'krbprincipalname' ]
         }).
-        details_facet({sections:[
-            {
-                name: 'details',
-                fields:['krbprincipalname',
+        details_facet({
+            sections: [
+                {
+                    name: 'details',
+                    fields: [
+                        'krbprincipalname',
                         {
-                            factory:IPA.service_name_widget,
+                            factory: IPA.service_name_widget,
                             name: 'service',
                             label: IPA.messages.objects.service.service,
                             read_only: true
                         },
                         {
-                            factory:IPA.service_host_widget,
+                            factory: IPA.service_host_widget,
                             name: 'host',
                             label: IPA.messages.objects.service.host,
                             read_only: true
-                        }]
-            },
-            {
-                name: 'provisioning',
-                fields:[{
-                    factory:IPA.service_provisioning_status_widget,
-                    name: 'provisioning_status',
-                    label: IPA.messages.objects.service.status
-                }]
-            },
-            {
-                name: 'certificate',
-                fields:[{
-                    factory:IPA.service_certificate_status_widget,
-                    name: 'certificate_status',
-                    label: IPA.messages.objects.service.status
-                }]
-            }]}).
+                        }
+                    ]
+                },
+                {
+                    name: 'provisioning',
+                    fields: [
+                        {
+                            factory: IPA.service_provisioning_status_widget,
+                            name: 'provisioning_status',
+                            label: IPA.messages.objects.service.status
+                        }
+                    ]
+                },
+                {
+                    name: 'certificate',
+                    fields: [
+                        {
+                            factory: IPA.service_certificate_status_widget,
+                            name: 'certificate_status',
+                            label: IPA.messages.objects.service.status
+                        }
+                    ]
+                }
+            ]
+        }).
         association_facet({
             name: 'managedby_host',
             add_method: 'add_host',
@@ -84,7 +93,7 @@ IPA.service_add_dialog = function(spec) {
     var that = IPA.add_dialog(spec).
         field(IPA.widget({
             name: 'krbprincipalname',
-            optional: true,
+            required: false,
             entity: spec.entity,
             hidden: true
         })).
@@ -106,7 +115,7 @@ IPA.service_add_dialog = function(spec) {
             editable: true,
             size: 10,
             entity: spec.entity,
-            param_info: { required: true }
+            required: true
         })).
         field(IPA.entity_select_widget({
             name: 'host',
@@ -114,7 +123,7 @@ IPA.service_add_dialog = function(spec) {
             other_field: 'fqdn',
             entity: spec.entity,
             label: IPA.messages.objects.service.host,
-            param_info: { required: true }
+            required: true
         })).
         field(IPA.checkbox_widget({
             name: 'force',
diff --git a/install/ui/sudo.js b/install/ui/sudo.js
index cca9d3edf2f124bc5f08f7f417f8fd686e935056..226ea8db42176bf410d8ea9443f96c3d10db10bd 100644
--- a/install/ui/sudo.js
+++ b/install/ui/sudo.js
@@ -181,8 +181,7 @@ IPA.sudorule_details_facet = function(spec) {
         });
 
         section.text({
-            name: 'cn',
-            read_only: true
+            name: 'cn'
         });
         section.textarea({
             name: 'description'
diff --git a/install/ui/test/data/ipa_init.json b/install/ui/test/data/ipa_init.json
index 94467ee99210ca1288fca6353ecc99e1f915153f..78b18ee118e563d4b118028ba36c41d7d8593a39 100644
--- a/install/ui/test/data/ipa_init.json
+++ b/install/ui/test/data/ipa_init.json
@@ -361,7 +361,6 @@
                     "true": "True",
                     "widget": {
                         "next": "Next",
-                        "optional": "Optional field: click to show",
                         "page": "Page",
                         "prev": "Prev",
                         "undo": "undo",
diff --git a/install/ui/user.js b/install/ui/user.js
index 60958cb43cf3f853c370554162600733f3d3d90d..bfda51d84b7e34bed5f429fcdd83e0b1ea6b4e43 100644
--- a/install/ui/user.js
+++ b/install/ui/user.js
@@ -141,9 +141,8 @@ IPA.entity_factories.user = function() {
         adder_dialog({
             fields: [
                 {
-                    factory : IPA.text_widget,
-                    optional: true,
-                    name:'uid'
+                    name: 'uid',
+                    required: false
                 },
                 'givenname',
                 'sn'
diff --git a/install/ui/widget.js b/install/ui/widget.js
index e34f2ea087837fe8484ec49102645cf7e8f7f722..1d3d31b38f12a8756d87e1b76aa3a294cfa96744 100644
--- a/install/ui/widget.js
+++ b/install/ui/widget.js
@@ -24,6 +24,7 @@
 /* REQUIRES: ipa.js */
 
 IPA.checkbox_column_width = 22;
+IPA.required_indicator = '*';
 
 IPA.widget = function(spec) {
 
@@ -40,7 +41,9 @@ IPA.widget = function(spec) {
 
     that.disabled = spec.disabled;
     that.hidden = spec.hidden;
-    that.optional = spec.optional || false;
+
+    // override the required flag in metadata
+    that.required = spec.required;
 
     // read_only is set when widget is created
     that.read_only = spec.read_only;
@@ -51,7 +54,7 @@ IPA.widget = function(spec) {
     that.width = spec.width;
     that.height = spec.height;
 
-    that.undo = typeof spec.undo == 'undefined' ? true : spec.undo;
+    that.undo = spec.undo === undefined ? true : spec.undo;
     that.join = spec.join;
 
     that.param_info = spec.param_info;
@@ -129,14 +132,38 @@ IPA.widget = function(spec) {
         }).appendTo(container);
     };
 
-    that.check_required = function(){
+    that.create_required = function(container) {
+        that.required_indicator = $('<span/>', {
+            'class': 'required-indicator',
+            text: IPA.required_indicator,
+            style: 'display: none; float: right;'
+        }).appendTo(container);
+    };
+
+    that.is_required = function() {
+        if (that.read_only) return false;
+        if (!that.writable) return false;
+
+        if (that.required !== undefined) return that.required;
+        return that.param_info && that.param_info.required;
+    };
+
+    that.set_required = function(required) {
+        that.required = required;
+
+        that.update_required();
+    };
+
+    that.update_required = function() {
+        if (that.required_indicator) {
+            that.required_indicator.css('display', that.is_required() ? 'inline' : 'none');
+        }
+    };
+
+    that.check_required = function() {
         var values = that.save();
-        if (!values || !values.length || values[0] === '' ) {
-            if (that.param_info &&
-                that.param_info.required &&
-                !that.optional &&
-                !that.read_only &&
-                that.writable) {
+        if (!values || !values.length || values[0] === '') {
+            if (that.is_required()) {
                 that.valid = false;
                 that.show_error(IPA.messages.widget.validation.required);
                 return false;
@@ -265,6 +292,7 @@ IPA.widget = function(spec) {
     };
 
     that.reset = function() {
+        that.update_required();
         that.update();
         that.validate();
         that.set_dirty(false);
@@ -838,6 +866,11 @@ IPA.checkbox_widget = function (spec) {
         that.input.attr('checked', value);
     };
 
+    // a checkbox will always have a value, so it's never required
+    that.is_required = function() {
+        return false;
+    };
+
     that.checkbox_save = that.save;
     that.checkbox_load = that.load;
 
@@ -1006,6 +1039,11 @@ IPA.radio_widget = function(spec) {
         }
     };
 
+    // a radio will always have a value, so it's never required
+    that.is_required = function() {
+        return false;
+    };
+
     // methods that should be invoked by subclasses
     that.radio_create = that.create;
     that.radio_save = that.save;
diff --git a/ipalib/plugins/internal.py b/ipalib/plugins/internal.py
index 8da38ee4d31e76b27065df39ca231d2c56d868c9..bf2417638adc3565dbef26da10c91f81141c5673 100644
--- a/ipalib/plugins/internal.py
+++ b/ipalib/plugins/internal.py
@@ -455,7 +455,6 @@ class i18n_messages(Command):
         "true": _("True"),
         "widget": {
             "next": _("Next"),
-            "optional": _("Optional field: click to show"),
             "page": _("Page"),
             "prev": _("Prev"),
             "undo": _("undo"),
-- 
1.7.5.1

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

Reply via email to