On 07/23/2013 06:09 PM, Petr Vobornik wrote: > On 07/22/2013 04:46 PM, Ana Krivokapic wrote: >> On 07/18/2013 09:47 AM, Petr Vobornik wrote: >>> On 07/17/2013 09:18 PM, Ana Krivokapic wrote: >>>> Hello, >>>> >>>> This patch addresses ticket https://fedorahosted.org/freeipa/ticket/3793. >>>> >>> >>> Hello, >>> >>> 1) IMO we should not create attribute which is just a negation of another. >>> >>> 2) We should add set_enabled method to base widget. Existing set_enabled >>> methods should use it and maintain widget output consistent with the >>> attribute >>> (ie. one should not directly set the attr and should use set_enabled >>> instead). >>> The method should be also callable when content is not yet created. >>> get_enabled methods might become unnecessary - one can get the state form >>> 'enabled' attribute. >>> >> >> The attached updated patch implements the following changes: >> >> 1) set_enabled method has been added to the base widget class. >> 2) get_enabled/is_enabled methods have been removed. >> 3) Widget classes that inherit from the base widget class override the >> set_enabled method where necessary. >> 4) Using 'enabled: true/false' in the widget definition should now work >> correctly for all types of widgets. >> >> > > Thanks. > > 1. set_enabled method in input_widget uses `that.input`. Input widget is a > base class which doesn't set the property and therefore we can't be certain > that the descendant will set it. Also it may break when you call > set_enabled(val) before create() . > > We should test for `that.input` presence. > > Same content-created test should be perform on other places: > widget.js:1017,1147,2006 > > 2. The changes in option_widget_base break disabling if user doesn't have > write-rights. (can be reproduced when navigated (by manual change of url) to > service in self-service. > > Note the differences between read_only, writable and enabled: > * read_only - reflects metadata > * writable - reflects ACL > * enabled - context specific > > read_only and writable don't offer edit interface (label instead of textbox). > Enabled controls disabled state of textbox. For some widgets the result might > be the same (radios, checkboxes). > > option_widget_base.set_enabled should be changed. The mixin overwrites the > original method and therefore doesn't set 'enabled' property. > > 3. multiple_choice_section.set_enabled should be renamed. It's related to > individual choices and not the widget itself. And then new set_enabled should > be added which would call the old one for each choice. > > 4. widget.js:3870 - not sure if it's needed but if so, one should also change > `action_clicked` method. >
All fixed. Updated patch attached. Thanks for the review. -- Regards, Ana Krivokapic Associate Software Engineer FreeIPA team Red Hat Inc.
From cd2bd3ad6f4596c56042a6e3d8c76596f7b4e6e8 Mon Sep 17 00:00:00 2001 From: Ana Krivokapic <[email protected]> Date: Wed, 17 Jul 2013 21:13:42 +0200 Subject: [PATCH] Honor 'enabled' option for widgets. https://fedorahosted.org/freeipa/ticket/3793 --- install/ui/src/freeipa/association.js | 1 - install/ui/src/freeipa/dns.js | 3 +- install/ui/src/freeipa/facet.js | 2 +- install/ui/src/freeipa/rule.js | 2 - install/ui/src/freeipa/widget.js | 127 ++++++++++++++++++++++------------ 5 files changed, 85 insertions(+), 50 deletions(-) diff --git a/install/ui/src/freeipa/association.js b/install/ui/src/freeipa/association.js index c60c7b8afe9c16ae55e5147574664c60afc43d3e..ad427d66b6b98119b2eb577ae98e4b7c2f1a6932 100644 --- a/install/ui/src/freeipa/association.js +++ b/install/ui/src/freeipa/association.js @@ -530,7 +530,6 @@ IPA.association_table_widget = function (spec) { $('.action-button', that.table).addClass('action-button-disabled'); that.unselect_all(); } - that.enabled = enabled; }; that.select_changed = function() { diff --git a/install/ui/src/freeipa/dns.js b/install/ui/src/freeipa/dns.js index b4085fea8b792e7f642a10373207916886ff50be..0a0fd3f85b33f51c474f3e6a47cca00ae9ffcfe9 100644 --- a/install/ui/src/freeipa/dns.js +++ b/install/ui/src/freeipa/dns.js @@ -603,7 +603,7 @@ IPA.dnszone_adder_dialog = function(spec) { var zone = zone_w.save()[0] || ''; var ns = ns_w.save()[0] || ''; - var zone_is_reverse = !zone_w.is_enabled() || + var zone_is_reverse = !zone_w.enabled || ends_with(zone, '.in-addr.arpa.') || ends_with(zone, '.ip6.arpa.'); var relative_ns = true; @@ -1767,7 +1767,6 @@ IPA.dns.record_type_table_widget = function(spec) { $('.action-button', that.table).addClass('action-button-disabled'); that.unselect_all(); } - that.enabled = enabled; }; that.select_changed = function() { diff --git a/install/ui/src/freeipa/facet.js b/install/ui/src/freeipa/facet.js index 37106e22f44b2fb50fc79b8183cc62e9eb35b6e4..b01452dd718b894ecb66d29f70242779ff75cfa4 100644 --- a/install/ui/src/freeipa/facet.js +++ b/install/ui/src/freeipa/facet.js @@ -1940,7 +1940,7 @@ exp.action_button_widget = IPA.action_button_widget = function(spec) { }; that.set_enabled = function(enabled) { - that.enabled = enabled; + that.widget_set_enabled(enabled); if (that.button_element) { if (enabled) { diff --git a/install/ui/src/freeipa/rule.js b/install/ui/src/freeipa/rule.js index f573b56d469ac8c6652f8e49c66fd6fd1259db90..332342bf8f7806da1c77a61d2da8677944657296 100644 --- a/install/ui/src/freeipa/rule.js +++ b/install/ui/src/freeipa/rule.js @@ -112,8 +112,6 @@ IPA.rule_association_table_widget = function(spec) { that.external = spec.external; - that.enabled = spec.enabled !== undefined ? spec.enabled : true; - that.setup_column = function(column, div, record) { var suppress_link = false; if (that.external) { diff --git a/install/ui/src/freeipa/widget.js b/install/ui/src/freeipa/widget.js index 06fcef563ca416e6e3e1cc454f2e1dd665c68f26..739abcde7071d567865358bd1e3b7e7f7c394d8a 100644 --- a/install/ui/src/freeipa/widget.js +++ b/install/ui/src/freeipa/widget.js @@ -51,6 +51,7 @@ IPA.widget = function(spec) { that.measurement_unit = spec.measurement_unit; that.entity = IPA.get_entity(spec.entity); //some old widgets still need it that.facet = spec.facet; + that.enabled = spec.enabled === undefined ? true : spec.enabled; that.create = function(container) { container.addClass('widget'); @@ -60,6 +61,10 @@ IPA.widget = function(spec) { that.clear = function() { }; + that.set_enabled = function(value) { + that.enabled = value; + }; + that.set_visible = function(visible) { if (visible) { @@ -81,6 +86,7 @@ IPA.widget = function(spec) { }; that.widget_create = that.create; + that.widget_set_enabled = that.set_enabled; return that; }; @@ -200,6 +206,14 @@ IPA.input_widget = function(spec) { } }; + that.set_enabled = function(value) { + that.widget_set_enabled(value); + + if (that.input) { + that.input.prop('disabled', !value); + } + }; + that.on_value_changed = function() { var value = that.save(); that.value_changed.notify([value], that); @@ -257,7 +271,6 @@ IPA.text_widget = function(spec) { that.input = $('<input/>', { type: that.input_type, name: that.name, - disabled: that.disabled, size: that.size, title: that.tooltip, keyup: function() { @@ -274,6 +287,7 @@ IPA.text_widget = function(spec) { } that.create_error_link(container); + that.set_enabled(that.enabled); }; that.update = function(values) { @@ -303,15 +317,6 @@ IPA.text_widget = function(spec) { } }; - that.is_enabled = function(value) { - return !that.input.prop('disabled'); - }; - - that.set_enabled = function(value) { - - that.input.prop('disabled', !value); - }; - that.clear = function() { that.input.val(''); that.display_control.text(''); @@ -474,7 +479,8 @@ IPA.multivalued_widget = function(spec) { name: that.name+'-'+row_index, undo: that.undo || row.is_new, read_only: that.read_only, - writable: that.writable + writable: that.writable, + enabled: that.enabled }); row.widget.create(row.container); @@ -756,6 +762,8 @@ IPA.option_widget_base = function(spec, that) { that._child_widgets.push(option.widget); } } + + option.enabled = spec.enabled === undefined ? true : spec.enabled; option.label = text.get(option.label); option.combine_values = option.combine_values === undefined ? true : !!option.combine_values; @@ -794,11 +802,13 @@ IPA.option_widget_base = function(spec, that) { that._create_option = function(option, container) { var input_name = that.get_input_name(); var id = that._option_next_id + input_name; + var enabled = that.enabled && option.enabled; $('<input/>', { id: id, type: that.input_type, name: input_name, + disabled: !enabled, value: option.value, title: option.tooltip || that.tooltip, change: that.on_input_change @@ -958,7 +968,7 @@ IPA.option_widget_base = function(spec, that) { // uncheck all inputs check(that._selector, true /*uncheck*/); - var writable = !that.read_only && !!that.writable; + var writable = !that.read_only && !!that.writable && that.enabled; if (!that.nested) { that.set_enabled(writable); } @@ -1011,9 +1021,18 @@ IPA.option_widget_base = function(spec, that) { }; that.set_enabled = function(enabled, clear) { + if (that.widget_set_enabled) { + that.widget_set_enabled(enabled); + } else { + that.enabled = enabled; + } + + if (that._selector && that.container) { + $(that._selector, that.container).prop('disabled', !enabled); + } - $(that._selector, that.container).prop('disabled', !enabled); if (!enabled && clear) that.clear(); + for (var i=0; i<that._child_widgets.length;i++) { that._child_widgets[i].set_enabled(enabled, clear); } @@ -1074,7 +1093,7 @@ IPA.checkbox_widget = function (spec) { spec.input_type = spec.input_type || 'checkbox'; if (!spec.options) { - spec.options = [ { value: checked, label: '' } ]; + spec.options = [ { value: checked, label: '', enabled: spec.enabled } ]; } if (spec.checked) spec.default_value = spec.checked; @@ -1137,6 +1156,15 @@ IPA.select_widget = function(spec) { } that.create_error_link(container); + that.set_enabled(that.enabled); + }; + + that.set_enabled = function(value) { + that.widget_set_enabled(value); + + if (that.select) { + that.select.prop('disabled', !value); + } }; that.create_options = function() { @@ -1231,7 +1259,6 @@ IPA.textarea_widget = function (spec) { name: that.name, rows: that.rows, cols: that.cols, - disabled: that.disabled, readOnly: !!that.read_only, title: that.tooltip, keyup: function() { @@ -1250,6 +1277,7 @@ IPA.textarea_widget = function (spec) { } that.create_error_link(container); + that.set_enabled(that.enabled); }; that.save = function() { @@ -1786,6 +1814,8 @@ IPA.table_widget = function (spec) { name: 'total_pages' }).appendTo(that.pagination_control); } + + that.set_enabled(that.enabled); }; that.prev_page = function() { @@ -1994,7 +2024,12 @@ IPA.table_widget = function (spec) { }; that.set_enabled = function(enabled) { - $('input[name="'+that.name+'"]', that.table).prop('disabled', !enabled); + that.widget_set_enabled(enabled); + + var input = $('input[name="'+that.name+'"]', that.table); + if (input) { + input.prop('disabled', !enabled); + } }; that.clear = function() { @@ -2122,7 +2157,6 @@ IPA.attribute_table_widget = function(spec) { $('.action-button', that.table).addClass('action-button-disabled'); that.unselect_all(); } - that.enabled = enabled; }; that.select_changed = function() { @@ -2489,6 +2523,7 @@ IPA.combobox_widget = function(spec) { } that.create_error_link(container); + that.set_enabled(that.enabled); }; that.on_no_close = function() { @@ -2633,7 +2668,7 @@ IPA.combobox_widget = function(spec) { }; that.open = function() { - if (!that.read_only) { + if (!that.read_only && that.enabled) { that.list_container.css('visibility', 'visible'); } }; @@ -2989,33 +3024,19 @@ IPA.button_widget = function(spec) { style: that.style, click: that.on_click }).appendTo(container); - }; - that.get_enabled = function() { - - var enabled = true; - - if (that.button) { - enabled = that.button.hasClass(that.disabled_class); - } - - return enabled; + that.set_enabled(that.enabled); }; that.set_enabled = function(enabled) { + that.widget_set_enabled(enabled); - enabled ? that.enable() : that.disable(); - }; - - that.enable = function() { - if (that.button) { - that.button.removeClass(that.disabled_class); - } - }; - - that.disable = function() { if (that.button) { - that.button.addClass(that.disabled_class); + if (enabled) { + that.button.removeClass(that.disabled_class); + } else { + that.button.addClass(that.disabled_class); + } } }; @@ -3313,6 +3334,8 @@ IPA.multiple_choice_section = function(spec) { choice = choices[i]; that.create_choice(choice); } + + that.set_enabled(that.enabled); }; that.create_choice = function(choice) { @@ -3353,6 +3376,7 @@ IPA.multiple_choice_section = function(spec) { id: radio_id, value: choice.name, checked: enabled, + disabled: !that.enabled, change: function() { that.select_choice(this.value); } @@ -3375,11 +3399,11 @@ IPA.multiple_choice_section = function(spec) { for (i=0; i<that.choices.values.length; i++) { choice = that.choices.values[i]; enabled = choice.name === choice_name; - that.set_enabled(choice, enabled); + that.set_choice_enabled(choice, enabled); } }; - that.set_enabled = function (choice, enabled) { + that.set_choice_enabled = function (choice, enabled) { var i, field_name, field, fields, required; @@ -3395,7 +3419,21 @@ IPA.multiple_choice_section = function(spec) { } }; + that.set_enabled = function(value) { + var i, choice; + + that.widget_set_enabled(value); + + for (i=0; i<that.choices.values.length; i++) { + choice = that.choices.values[i]; + that.set_choice_enabled(choice, value); + } + }; + that.init_enabled = function() { + if (!that.enabled) { + return; + } var i, choice; @@ -3790,7 +3828,8 @@ IPA.sshkey_widget = function(spec) { dialog.textarea = $('<textarea/>', { 'class': 'certificate', - readonly: that.read_only + readonly: that.read_only, + disabled: !that.enabled }).appendTo(dialog.container); var key = that.key.key || ''; @@ -3868,7 +3907,7 @@ IPA.action_panel = function(spec) { if (!action.visible) return; classes = ['action']; - state = action.enabled ? 'enabled' : 'disabled'; + state = action.enabled && that.enabled ? 'enabled' : 'disabled'; classes.push(state); li = $('<li/>'); @@ -3908,7 +3947,7 @@ IPA.action_panel = function(spec) { that.action_clicked = function(action) { - if (!action.enabled || !action.visible) return; + if (!that.enabled || !action.enabled || !action.visible) return; action.execute(that.facet); }; -- 1.8.1.4
_______________________________________________ Freeipa-devel mailing list [email protected] https://www.redhat.com/mailman/listinfo/freeipa-devel
