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

Endi Sukma Dewata edewata at redhat.com
Wed Sep 21 20:10:36 UTC 2011


On 9/21/2011 6:50 AM, Petr Vobornik wrote:
> 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.

Fixed.

> 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).

Fixed. The dialog fields don't need undo, so the text() needs to be 
overridden to disable undo. This can be improved again later.

> 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.

Fixed. I don't think the "if (valid)" is necessary because if the 
section is invalid it won't go there.

We might actually be able to call the dialog.is_valid() instead of 
calling section.is_valid() individually. This can be improved again later.

> 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).

That's the plan after we can get a consistent code. I tried using the 
same code but ran into some issues, so we need to figure that out first.

> 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).

Fixed. Yes, we should have a ticket to remove all inline CSS styles.

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

I think we do need the spacing between FQDN and the IP address because 
otherwise the interface will be too cluttered.

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

Removed the code. We will use that in 3.0.

Since we need to get this done soon I've asked Adam to help review this. 
But feel free to review this too even if it's already pushed. Thanks!

-- 
Endi S. Dewata
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-edewata-0271-2-Modified-dialog-to-use-sections.patch
Type: text/x-patch
Size: 48600 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20110921/4941df4f/attachment.bin>


More information about the Freeipa-devel mailing list