1561390 - Unaligned elements in the account manager

Status: RESOLVED FIXED

(MailNews Core :: Account Manager, defect)

Description

[:aceman]

Attachment: bug-TB-AM-alignment1.png

After removal of grid element in bug 1521483, some elements in the Account manager now do not align properly, the label and its corresponding textbox are not centered on the same line/row. See the attachments

Comment 1

[:aceman]

Attachment: bug-TB-AM-alignment1.png

Comment 2

[:aceman]

Attachment: bug-TB-AM-alignment2.png

Comment 3

[Khushil Mistry [:khushil324]]

Attachment: Bug-1561390_fixed_unaligned-elements.patch

Comment 4

[:aceman]

Comment on attachment 9074178 [details] [diff] [review]
Bug-1561390_fixed_unaligned-elements.patch

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

Thanks, but you can't hardcode heights like this. You never know how high the label's font will end up and how high the textbox in the other 'column' will be (which you do not height-limit in this patch). Yes, you may have some css rules in accountManage.css, but those differ by platform and may still end up with unexpected size.

The most trivial (but ugly) solution I could see is to add some class to the css files that limits the height to some appropriate height (e.g. 1.5em) and apply that class for BOTH the label and the corresponding textbox in the same visual row.

The <grid> element ensured this automatically, so dropping it seems like a regression.
Is there no replacement in the HTML world?

Comment 5

[Khushil Mistry [:khushil324]]

Attachment: Bug-1561390_fixed_unaligned-elements.patch

Comment 6

[Richard Marti (:Paenglab)]

You could try to use a html:table like I did in Calendar/General.

Comment 7

[Khushil Mistry [:khushil324]]

Attachment: Bug-1561390_fixed_unaligned-elements.patch

Comment 8

[Khushil Mistry [:khushil324]]

You could try to use a html:table like I did in Calendar/General.

Yess, I have tried it with few of the de-grid tasks. Do you want it as a submitted patch or want to have as the html:table? I use hbox and vbox in trivial solutions. I generally use html:table, when we don't have any other options available.

Comment 9

[:aceman]

Comment on attachment 9074611 [details] [diff] [review]
Bug-1561390_fixed_unaligned-elements.patch

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

Somehow this didn't work in the identity settings.
Please try the html table version.

Comment 10

[Khushil Mistry [:khushil324]]

Attachment: Bug-1561390_fixed_unaligned-element.patch

Comment 11

[:aceman]

Comment on attachment 9076353 [details] [diff] [review]
Bug-1561390_fixed_unaligned-element.patch

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

Thanks. This seems to solve the vertical alignment of the elements.
But now the elements aren't packed to the left to use as low space as needed. This is seen best on a NNTP server's Server settings pane, where there is only "Connection Security" and its menulist is far right of it.
Is there an attribute on html table to make it NOT try to fill 100% of the document width?

::: mailnews/base/prefs/content/am-identity-edit.xul
@@ +67,5 @@
>                  <label value="&name.label;" control="identity.fullName"
>                         accesskey="&name.accesskey;"/>
> +              </html:td>
> +              <html:td>
> +                <textbox id="identity.fullName" size="30" style="width:100%;"/>

Why are these width:100% needed?

::: mailnews/base/search/content/searchTerm.js
@@ +219,4 @@
>  function getSearchRowIndexForElement(aElement) {
>    var listItem = aElement;
>  
> +  console.log(listItem);

I assume leftover debugging remnants.

Comment 12

[Khushil Mistry [:khushil324]]

Attachment: Bug-1561390_fixed_unaligned-element.patch

Comment 13

[Khushil Mistry [:khushil324]]

(In reply to :aceman from comment #11)

Thanks. This seems to solve the vertical alignment of the elements.
But now the elements aren't packed to the left to use as low space as
needed. This is seen best on a NNTP server's Server settings pane, where
there is only "Connection Security" and its menulist is far right of it.
Is there an attribute on html table to make it NOT try to fill 100% of the
document width?

This low space between label and menulist/box is set up by table so I don't think we can do anything with that.

Why are these width:100% needed?

To make the textbox and menulist covering the remaining width of the dialogue box. We can have less width for menulist (like 75%) but I found 100% width looking good.

Comment 14

[:aceman]

Comment on attachment 9076900 [details] [diff] [review]
Bug-1561390_fixed_unaligned-element.patch

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

I found how to do what I mean:
https://stackoverflow.com/questions/11413365/how-to-make-table-cell-shrink-according-to-the-content

You also need to add 'style="width:100%;"' to the second <td> of each row to make it expand and the first column (td) will shrink to only the space it really needs.

r+ with that added :)

Comment 15

[Khushil Mistry [:khushil324]]

Attachment: Bug-1561390_fixed_unaligned-element.patch

Comment 16

[:aceman]

Comment on attachment 9077949 [details] [diff] [review]
Bug-1561390_fixed_unaligned-element.patch

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

::: mailnews/base/prefs/content/am-identity-edit.xul
@@ +61,4 @@
>        <vbox flex="1" name="settings">
>          <groupbox>
>            <label class="header">&publicData.label;</label>
> +          <html:table style="width:200px;">

Why this now? Never hardcode aboslute sizes needlessly, what if somebody sets very small font?
The 100% on the second cell should be enough.

Comment 17

[:aceman]

Attachment: 1561390.patch final

Comment 18

[:aceman]

I removed the width:200px, still looks good to me.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/48e3344da003
fixed unaligned elements in the account manager. r=aceman

Comment 20

[Jorg K (CEST = GMT+2)]

TB 68 beta 5:
https://hg.mozilla.org/releases/comm-beta/rev/bbc19c7449026616efc289158a968da7b3e7b390

TB 69 beta:
https://hg.mozilla.org/releases/comm-beta/rev/099ce2ae99a078da668f572217509fb2ab7f0e46