Bug 662337

Summary: re-implement a better solution to editable fields in dynamic forms
Product: [Other] RHQ Project Reporter: John Mazzitelli <mazz>
Component: Core UIAssignee: John Mazzitelli <mazz>
Status: CLOSED CURRENTRELEASE QA Contact: Mike Foley <mfoley>
Severity: medium Docs Contact:
Priority: low    
Version: 3.0.0   
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2013-09-02 07:19:52 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description John Mazzitelli 2010-12-11 20:24:04 UTC
Currently, we use ToggleableTextItem in conjunction with EnhancedDynamicForm to support "editable" static fields. However, the implementation isn't ideal because the "ghost pencil" appears late after hovering and it hangs around for 5 seconds after the mouse goes away from the field, and only after the 5s does it disappear.

Recommend using a different method so the pencil (aka edit icon) appears and disappears in a more "snappy" fashion. We should also provide a set of "ok/cancel" icons while editing, to allow not only a "enter" keystroke to set the value, but allow pressing the "ok" button to do it do. The cancel button lets you explicitly go back to static view without committing the change (essentially "reverting back" to the original value).

Comment 1 John Mazzitelli 2010-12-11 20:30:57 UTC
removed the old impl and added the new one. EditableFormItem is now the new FormItem you can use to support this feature. The edit icon comes and goes much better now (as soon as you hover/unhover from the value). You get a set of ok/cancel icons as well.

The new implementation now also supports boolean values (which was needed for the group recursive setting - see bug 661887). Using CheckboxEditableFormItem allows you to tell the UI you want to show a checkbox for the editable field, as opposed to the default TextItem.

There is some more enhancements we can do with this:

1) nothing happens if you lose focus from the edit field. What do we want it to do? Cancel or Save? Right now, it does nothing - you have to hit "enter", or click the ok or cancel icons to get it to do something.

2) the width of the field isn't entirely correct. It looks like the width of the item isn't as wide as the dynamic form column. Need to figure out the proper API calls to make so the item expands to fit the entire width of the form.

Comment 2 John Mazzitelli 2010-12-13 16:00:31 UTC
(In reply to comment #1)
> 2) the width of the field isn't entirely correct. It looks like the width of
> the item isn't as wide as the dynamic form column. Need to figure out the
> proper API calls to make so the item expands to fit the entire width of the
> form.

This is now fixed. I had to call "setNumCols(1)" on the inner dynamic form (see git commit 20347e5bafd43473101b5a5f27977ba9a1ad3690).

This can be closed now. The only thing we have left to address in the future is "should we do anything on blur". Right now, the user is required to explicitly tell us the editing is done, either via hitting enter key or clicking one of the ok/cancel icon. This is OK and works fine for now.

Comment 3 Heiko W. Rupp 2013-09-02 07:19:52 UTC
Bulk closing of issues that were VERIFIED, had no target release and where the status changed more than a year ago.