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.
--
Petr Vobornik
_______________________________________________
Freeipa-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/freeipa-devel
