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
