On 06/01/2012 02:40 AM, Endi Sukma Dewata wrote:
ACK. Looks good.

Pushed to master.


Some comments:

1. I suppose the select_action will always be the first action in any
header_actions, and the action doesn't actually do anything. You might
want to consider the '-- select action --' as part of the
IPA.action_list_widget and add it automatically in init_options(), that
way it doesn't have to be defined explicitly in all header_actions.

Can be. If I do it, I will do it configurable with default to use the '-- select action --'.


2. Ideally the Enable/Disable/Delete actions should only be enabled if
the user has rights to do that, but this depends on ticket #2187.

Yes. Also, I think enable and disable don't need #2187.


3. I noticed that the second argument of observer's notify() is always
the object that owns the observer. For example:

that.observer = IPA.observer();
that.observer.notify([arg], that);

Since the context doesn't change would it make sense to store the
context in the observer itself?

that.observer = IPA.observer({ context: that });

The arguments can also be passed as varargs:

that.observer.notify(arg);
that.observer.notify(arg1, arg2);

I like this idea. It simplifies things.



On 5/24/2012 3:54 AM, Petr Vobornik wrote:
This bunch of patches implements new concept: action panel and it's
implementation in user page.

First two patches refactorizes current action-list/control-buttons code
to prepare ground for following patches. Sorry for added review work
(could be done this way earlier).

Patch descriptions:

[PATCH] 138 Refactored action list and control buttons to use shared
list of actions

This is a first step for implementing action panels which will also use
the shared list of actions.

This effort changes the way how action list and control buttons are
defined. First all actions are defined on facet level - attribute
'actions' in spec file. Implementation of action list widget is not
specified on facet level. It is left in facet header. A list of action
names used in action list can be now specified in facet spec in
'header_actions' attribute.
Control buttons use similar concept. Facet by default is using
control_buttons_widget. Details and search facet are defining their own
default actions (refresh/add/remove/update/reset). Additional buttons
can be defined as array of action names on facet level in
control_buttons attribute.

state_evaluators and state_listeners were united. They are called
state_evaluators but they uses state_listener concept, they are attached
to an event. For former state_evaluator the event is post_load. They are
defined in spec in state attribute. State object purpose is to aggregate
states from all state evaluators. It offers changed event to which can
other objects subscribe. It also has summary evaluator which evaluation
conditions. Summary evaluator creates summary status with human readable
description. It can be used by facet header.

https://fedorahosted.org/freeipa/ticket/2248


[PATCH] 139 Refactored entities to use changed actions concept

It's continuation of previous refactoring effort. This part is changing
specs in entities to used changed concept.

https://fedorahosted.org/freeipa/ticket/2248

[PATCH] 140 Action panel

This patch implements action panel. Action panel is a box located in
facet details section which contains actions related to that
object/section.

In spec file can be configured actions and title used in action panel.
Default title is 'Actions'. Actions are specified by their name. They
have to be defined in action collection in facet.

https://fedorahosted.org/freeipa/ticket/2248


[PATCH] 141 User password widget modified.

Currently the user password is shown as follows in the details page:
Password: Reset Password

This is inconsistent with the rest of the page because the 'Reset
Password' is an action, not the value of the password.

Now password is shown as follows:
Password: ******* (if set)
Password: (if not set)

Reset password link was removed as well the dialog for reset password
was removed from password widget. The dialog was moved to its own object
and can be now showed independently. An action for showing this dialog
should be created.

https://fedorahosted.org/freeipa/ticket/2248


[PATCH] 142 Action panel for user

This patch adds action panel to user account section. The panel contain
an action for reseting user password.

https://fedorahosted.org/freeipa/ticket/2248

[PATCH] 143 Added missing i18n in action list and action panel

This patch adds strings to internal.py which were not translated in
action list/panel patches.

https://fedorahosted.org/freeipa/ticket/2248


[PATCH] 144 Add shadow to dialog

This patch adds shadow to dialog used in Web UI. It looks cooler.

https://fedorahosted.org/freeipa/ticket/2248

note: I didn't want to create new ticket just for this minor visual
enhancement.


[PATCH] 145 Enable reset password action according to attribute
perrmission

This patch creates state_evaluator which creates permission states for
defined attribute. The state format is: attributeName_permissionChar.

This evaluator is used for user_password attribute and it control
enabling/disabling of related action in user account action panel.

https://fedorahosted.org/freeipa/ticket/2318



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




--
Petr Vobornik

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

Reply via email to