[Freeipa-devel] [PATCH] 0229-automount-delete-key

[Endi Sukma Dewata]

On 5/31/2011 11:59 AM, Adam Young wrote:
>

Some issues:

1. The labels for mount and parentmap fields in automount map are
    missing.

2. The mount and parentmap fields in automount map adder dialog shows
    undo button. This can be fixed by adding "undo: false" parameter.

3. The Delete button remains disabled after selecting some automount
    keys to be deleted.

4. Clicking 'Back to List' when viewing a map brings you back to list
    of locations. Is this still intentional? Perhaps the label should be
    changed to 'Back to Locations' or simply hidden.

5. The conditional fields in IPA.dialog are a little bit limited
    because there is only one set of conditional fields which has to be
    enabled/disabled together. It might be better to replace the
    'conditional' boolean paramter into 'field_group' then replace the
    enable/disable methods to accept a field group. This could be done
    later.

6. The add() in IPA.automountmap_adder_dialog is probably unnecessary
    because it's only calling the superclass's add() method.

7. The following assignment in automount.js line 158:

      var input = $('input[name="'+that.name+'"]', that.container);

    could be moved to the beginning of the method to avoid reexecuting
    the same jQuery selector.

8. In dialog.js line 626 and search.js line 253, the hasOwnProperty()
    invocations are probably redundant because the key is obtained from
    the object itself, so that method will always return true.

9. The "if (first)" statement in dialog.js line 628 will only append a
    comma after the first key-value pair and nothing after that. This
    statement probably should have been "if (!first)" and moved to the
    beginning of the loop.

10. The 3rd level tab for automount key was removed. At this point does
     it makes sense to remove the 3rd level tabs completely?

11. The option values for automount map adder dialog could be
     simplified to "direct" and "indirect".

-- 
Endi S. Dewata