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

Adam Young ayoung at redhat.com
Fri Feb 4 01:36:53 UTC 2011


On 02/03/2011 04:18 PM, Endi Sukma Dewata wrote:
> On 2/3/2011 8:41 AM, Adam Young wrote:
>> 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.
>
> Attached is an updated patch. The line-level undo has been added.
>
>> 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.
>
> Line-out removal has been added as well. Please see the new patch 
> description. Thanks!
>
ACK, pushed to master.  Nicely done.




More information about the Freeipa-devel mailing list