[Freeipa-devel] [PATCH] 271 Modified dialog to use sections.

[Petr Vobornik]

On 09/15/2011 02:10 AM, Endi Sukma Dewata wrote:
> The IPA.dialog has been modified to store sections instead of fields.
> If there is no sections specified, it will create a default section.
>
> The adder dialog for automount map has been modified such that the
> fields related to indirect map are stored in a section which will
> only be visible when the map type is set to indirect.
>
> The adder dialog for host has been modified such that it uses a
> custom section for hostname and DNS zone and standard section for
> the other fields.
>
> Ticket #1394
>
>
>
> _______________________________________________
> Freeipa-devel mailing list
> Freeipa-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel

1) dialog.js:128 is_valid method should use section.is_valid method - no 
need to reimplement the same thing. On top of that, section.is_valid 
method checks required fields.

2) dialog.js:44 init() - uses the same code as details section. Wouldn't 
be better to split init method in details section to two parts?:
1: add_fields(spec) - which would accept array of field spec objects.
2: private init function which would call the add_fields method
Then we could make a get_section method in dialog which would return 
last section (same code as in add_field). At last we would call 
section.add_fields(fields).

3) add.js_:44 add() method. I know, there is a TODO comment, but I 
think, we could make validation almost consistent right now. Plain loop 
through sections like the one in details.js:618 and additional if(valid) 
check before command argument construction would do the trick.

I'm thinking if we should extract code for creating command(arguments 
and options) into separate object. Something like 
IPA.command_builder.add_arguments_sections(command, sections).

4) host.js:208,217: we should avoid using purely visual inline css 
styles. They should be replaced by class (if cannot be achieved by other 
selector) and styled in css file. This doesn't concern functional styles 
(animations, resizing, hiding, showing).

5) In host adder dialog. Is the margin between fqdn and other section 
OK? I don't mind it, just wondering.

6) group.js:100 param_info contains invalid string "Create as a 
non-POSIX group" for nonposix checkbox usage.



-- 
Petr Vobornik