Closed Bug 1561390 Opened 1 year ago Closed 1 year ago

Unaligned elements in the account manager

Categories

(MailNews Core :: Account Manager, defect)

All
Linux
defect
Not set

Tracking

(thunderbird68 fixed, thunderbird69 fixed, thunderbird70 fixed)

RESOLVED FIXED
Thunderbird 70.0
Tracking Status
thunderbird68 --- fixed
thunderbird69 --- fixed
thunderbird70 --- fixed

People

(Reporter: aceman, Assigned: khushil324)

References

(Regression)

Details

(Keywords: regression)

Attachments

(3 files, 7 obsolete files)

Attached image bug-TB-AM-alignment1.png (obsolete) —

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

Attachment #9073908 - Attachment is obsolete: true
Flags: needinfo?(khushil324)
Assignee: nobody → khushil324
Flags: needinfo?(khushil324)
Attachment #9074178 - Flags: review?(acelists)
Status: NEW → ASSIGNED
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?
Attachment #9074178 - Flags: review?(acelists) → review-
Attachment #9074178 - Attachment is obsolete: true
Attachment #9074603 - Flags: review?(acelists)
Attachment #9074603 - Flags: review?(acelists)

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

Attachment #9074603 - Attachment is obsolete: true
Attachment #9074611 - Flags: review?(acelists)

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.

Flags: needinfo?(acelists)
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.
Attachment #9074611 - Flags: review?(acelists)
Attachment #9074611 - Attachment is obsolete: true
Attachment #9076353 - Flags: review?(acelists)
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.
Attachment #9076353 - Flags: feedback+
Attachment #9076353 - Attachment is obsolete: true
Attachment #9076353 - Flags: review?(acelists)
Attachment #9076900 - Flags: review?(acelists)

(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 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 :)
Attachment #9076900 - Flags: review?(acelists) → review+
Attachment #9076900 - Attachment is obsolete: true
Attachment #9077949 - Flags: review+
Keywords: checkin-needed
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.
Attachment #9077949 - Flags: review+
Flags: needinfo?(acelists) → needinfo?(khushil324)
Keywords: checkin-needed
Attachment #9077949 - Attachment is obsolete: true
Attachment #9077961 - Flags: review+

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

Flags: needinfo?(khushil324)
Keywords: checkin-needed

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/48e3344da003
fixed unaligned elements in the account manager. r=aceman

Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 70.0
You need to log in before you can comment on or make changes to this bug.