1544303 - remove grid usage from comm/mailnews/base/content/virtualFolderProperties.xul

Status: RESOLVED FIXED

(Thunderbird :: General, task)

Comment 1

[Khushil Mistry [:khushil324]]

Attachment: Bug-1544303-remove_grid_virtualFolderProperties.patch

Comment 2

[Magnus Melin [:mkmelin]]

Comment on attachment 9058159 [details] [diff] [review]
Bug-1544303-remove_grid_virtualFolderProperties.patch

Review of attachment 9058159 [details] [diff] [review]:
-----------------------------------------------------------------

The items are now in quite the wrong places. Please compare with trunk.

Comment 3

[Khushil Mistry [:khushil324]]

Attachment: after.png

Screenshot after applying patch

Comment 4

[Khushil Mistry [:khushil324]]

Attachment: before.png

Screenshot before applying patch

Comment 5

[Khushil Mistry [:khushil324]]

(In reply to Magnus Melin [:mkmelin] from comment #2)

The items are now in quite the wrong places. Please compare with trunk.

I don't find any problem with this. Can you elaborate a little what was the exact problem?

Comment 6

[Magnus Melin [:mkmelin]]

That's not the right dialog... Right click an existing search folder to see it. It's pretty obvious once you get there.

Comment 7

[Khushil Mistry [:khushil324]]

Attachment: Bug-1544303-remove_grid_virtualFolderProperties.patch

Comment 8

[Magnus Melin [:mkmelin]]

Comment on attachment 9060006 [details] [diff] [review]
Bug-1544303-remove_grid_virtualFolderProperties.patch

Review of attachment 9060006 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, r=mkmelin

::: mailnews/base/content/virtualFolderProperties.xul
@@ +45,5 @@
> +    <vbox>
> +      <hbox flex="1" align="center">
> +        <label value="&name.label;"
> +               accesskey="&name.accesskey;"
> +               control="name"/>

where it fits the line width, it's more useful to have it all on one line

@@ +83,5 @@
> +                oncommand="chooseFoldersToSearch();"/>
> +      </hbox>
> +    </vbox>
> +    <vbox flex="2">
> +    </vbox>

is this needed?

Comment 9

[Khushil Mistry [:khushil324]]

Attachment: Screenshot 2019-04-23 at 8.55.35 PM.png

Without vbox, it will look something like this. Is it okay or we should continue with previous layout ?

Comment 10

[Magnus Melin [:mkmelin]]

Seems that just removes some extra space above "Configure the search criteria". That's fine I think, and preferable even.

Comment 11

[Khushil Mistry [:khushil324]]

(In reply to Magnus Melin [:mkmelin] from comment #10)

Seems that just removes some extra space above "Configure the search criteria". That's fine I think, and preferable even.

It is actually removing the 3rd Column which is empty like https://bugzilla.mozilla.org/show_bug.cgi?id=1544303#c3 so width of textbox and menubar will increase.

Comment 12

[Magnus Melin [:mkmelin]]

Right, I'd still go for it :)

Comment 13

[Khushil Mistry [:khushil324]]

Attachment: Bug-1544303-remove_grid_virtualFolderProperties.patch

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/8e40e1972fde
remove grid usage from virtualFolderProperties.xul. r=mkmelin