On 9/9/2014 10:48 AM, Petr Vobornik wrote:
[PATCH] 727 webui: hide empty fields and sections
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.
Respective fields are present only in corresponding object classes ->
there won't be an HTOP token with 'clock offset'. If they are present,
they have a default value. -> No false hiding -> Shouldn't be a problem.
ACK.
What do you think about setting `hide_empty_widgets` global setting to
True?
On a read-only page I think it's OK to hide empty fields. But on edit
page I don't think we should do that by default.
Maybe we should first clarify what is a read-only page. All details
pages are writable if user has the rights or read-only if he doesn't.
But mostly it depends on individual fields/attributes.
Read-only page is a page that does not provide editable interface for
all fields displayed in the page. For example, the fields in the
certificate details page are never editable, so suppose it has empty
optional fields (e.g. cert extensions?) we probably can hide those
fields without confusing users.
Other details pages have read-only and edit modes, so it probably makes
more sense to keep the structure identical in both modes so optional
fields like 'Job Title' will always be visible. However, it's also
possible to hide the empty fields in the read-only mode if necessary to
clean up the display.
Does "empty" mean "unset", or "blank" like empty string/array? Does
"unset" always mean "non-editable and invisible" and "blank" always mean
"editable and visible but it's just empty"? If this definition isn't
strictly followed there could be an editable field that accidentally
gets hidden because it's empty.
My intent was to show everything what user can edit. And hide fields:
1. for which he doesn't have read rights
2. have no (undefined) or empty('') value
3. are explicitly hidden by other logic - not related to this patch
When thinking about #1. Maybe we should base it on a presence or rather
not presence of a specific objectclass, rather than on a value. That way
it would be less confusing for newcomers. The logic is: if fields is
bound to an object class and the class is not present (that also means
we don't have attribute level rights) -> hide it.
That would be hiding/displaying based on structural reason instead of
user rights. How about an attribute that exists but the user doesn't
have the read permission? I think we need to check both structural and
user rights.
#2 is questionable. IMHO it would require user tests. Also hiding on ''
value might not be always desirable. Nevertheless I like the behavior
but it may be caused by the fact that I already know what to expect.
Before we can make this behavior the default/global, how do we make sure
that optional fields such as 'Job Title' will always visible in the edit
page even if it's empty? I think to avoid unexpected behavior hiding
based on empty value should only be done in read-only page as I
mentioned above.
Generally if a field is supposed to be hidden in an edit page because of
other condition (e.g. token type), not because of the value, the code
should also use that condition instead of relying on the value being
unset. In this particular case, instead of:
if (field !== undefined) {
// display field
}
we probably should do:
var type = ...
if (type == 'hotp') {
// display hotp fields
} else { // totp
// display totp fields
}
I agree - use case #3.
This is actually hiding/displaying based on structural reason too. In
this case the structure is reflected in the (virtual) 'type' field.
4. If no "Owner" is specified when adding a token, it will default to
admin. Is this the intended behavior?
Sadly yes.
Maybe the adder dialog should show admin (or the current user?) as the
default owner instead of empty?
Note: all patches except for 727 are ACKed.
It's all ACKed now. Everything else can be handled separately.
--
Endi S. Dewata
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
