[Freeipa-devel] [PATCH] Added multi-valued text widget.

[Adam Young]

On 02/03/2011 07:57 AM, Endi Sukma Dewata wrote:
> A multi-valued text widget has been created to replace the old
> IPA.details_field. There are some differences:
>
> The old code was designed to handle all data types, so the code is
> incomplete and complex. The new code was designed to handle multi-
> valued text attributes only, so it's easier to maintain. There are
> already other widgets that can be used to handle other data types.
>
> In the old code, if an attribute contains multiple values there
> will be one undo link for each value. In the new code there will
> be only one undo link for the whole attribute.
>
> In the old code, when a value is removed, the value will be crossed
> out. In the new code when a value is removed the entire line will
> disappear.
>
>
> _______________________________________________
> Freeipa-devel mailing list
> Freeipa-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel

NACK.  Mostly good, but not sure I agree 100%.  Line level Undo we very 
specific for the multi values.  Undo should be for individuals, not for 
the overall.

I realize that this makes the logic a little bit harder if you want to, 
say,  abandon your changes on phonen numbers, but keep them for Title,  
it is hard to get the undo just right.

So:  Multi values should have an "undo all"  in addition to line level 
undo.

I'd like to leave the "line-out"  approach in there for removed entries 
as well.  A user can always repurpose a line, so there undo/redo will be 
valuable at the line level. For straight delete, I think it is valuable 
for the user to see the original value.


Also, it looks like the code for "create_remove_link" is still in 
IPA.details_field.  I'm guessing that this is dead code that should be 
removed.  At a minimum, it should be moved to the new widget.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20110203/2792c630/attachment.htm>