On 8/5/2014 6:31 AM, Petr Vobornik wrote:
patches 725-728 are preparation for 729 but they can also affect the rest of UI (intentional). 728 is useful for non-admins. We might want to discuss enabling of `hide_empty_widgets` flag in patch 727.


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

[PATCH] 720 webui: add measurement unit to token time step field
ACK.

[PATCH] 721 webui: better otp token type label
ACK.

[PATCH] 722 webui: add token from user page

Add 'Add OTP Token' action to user action menu.

This option is disabled in self-service when viewing other users.

ACK, but I'm just wondering if it would be more intuitive to define an enabled condition (you're an admin or editing your own user) rather than a disabled condition (you're in self-service mode but editing other user).

[PATCH] 723 webui: add i18n for the rest of QR code strings

ACK.

[PATCH] 724 webui: display fields based on otp token type

- in adder dialog


[PATCH] 725 webui: better value-change reporting

- widget save() save method should try to always return value even if read only - report value-change event with actual value to allow processing of the value

ACK.

[PATCH] 726 webui: widget initialization

- used `ctor_init` instead of `init` to avoid name collision with
  existing logic
- `ctor_init` is called right after widget instantiation. Basically support
  better inheritance for the old class system which doesn't have proper
  contructors

ACK.

[PATCH] 727 webui: hide empty fields and sections

Hide widgets without a value. Must be explicitly turned on. In widget by
`hidden_if_empty` flag. Or globally by `hide_empty_widgets` flag. Global
hiding can be individually turned off by `ignore_empty_hiding` flag.

In item #2 of ticket #4402 I was originally thinking the widget visibility would be determined by the token type. Any plan to add the token type field in the future? Will the "counter" field strictly have a value with HOTP only and "clock offset & interval" fields strictly have value with TOTP only? Do these fields contain the configured values or the effective values? I was just thinking maybe in the future some of these fields might be configured empty and they will use the default values instead. If that's not a problem then ACK.

[PATCH] 728 webui: hide non-readable fields

hide widgets if associated field had received attribute level rights
without 'r' right.

Explicit rights are required to avoid hiding of special widgets which
are not associated with any LDAP attribute.

ACK.

[PATCH] 729 webui: hide otp fields based on token type

- uses hide empty feature

Assuming patch #727 is fine then it's ACKed too.

Some other comments/questions:

1. The "Validity start/end" fields don't show the date/time format. When you type the first letter then it will show the validation message with the format. It's not a big deal, but it's not very intuitive because people might not know what letter to type in the first place. Usually the field label should indicate the format/unit and the validation message will only appear if the value entered doesn't match the format/unit.

2. The "Digits" field by itself sounds a bit weird. How about "Number of digits", or "OTP length" and add the unit in the value (e.g. 6 digits)?

3. The "Clock offset" field doesn't have a unit.

4. If no "Owner" is specified when adding a token, it will default to admin. Is this the intended behavior?

--
Endi S. Dewata

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

Reply via email to