1581754 - remove grid usage from comm/mail/components/compose/content/dialogs/EdInsertTOC.xul

Status: RESOLVED FIXED

(Thunderbird :: Message Compose Window, task)

Comment 1

[Khushil Mistry [:khushil324]]

Attachment: Bug-1581754_remove-grid-EdInsertTOC.patch

Comment 2

[Magnus Melin [:mkmelin]]

Comment on attachment 9093230 [details] [diff] [review]
Bug-1581754_remove-grid-EdInsertTOC.patch

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

Actually, this seems like it should be an html:table

::: mail/components/compose/content/dialogs/EdInsertTOC.xul
@@ +29,5 @@
> +      <hbox class="groupbox-title">
> +        <label class="header">&buildToc.label;</label>
> +      </hbox>
> +      <html:div class="grid-three-column">
> +        <html:div></html:div>

use something like "grid-column: 2 / 3"; instead.

@@ +31,5 @@
> +      </hbox>
> +      <html:div class="grid-three-column">
> +        <html:div></html:div>
> +        <html:div class="flex-items-center">
> +          <label value="&tag.label;"/>

just &tag.label;, no need for the label

Comment 3

[Khushil Mistry [:khushil324]]

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

Actually, this seems like it should be an html:table

I try to use grid-layout first. If it does not work because of odd margins and paddings, I switch to html:table. Here, grid-layout worked so I went with it.

Comment 4

[Khushil Mistry [:khushil324]]

Attachment: Bug-1581754_remove-grid-EdInsertTOC.patch

Comment 5

[Magnus Melin [:mkmelin]]

But the data is semantically tabular data. If you flip the axis, it still works. A grid layout that would try to adapt for responsivness would easily make the data non-readable. (You can't have it any other way than 3 elements on the one row, try changing to a grid of 2 or 4, and nobody understands a thing there.)

Comment 6

[Khushil Mistry [:khushil324]]

Attachment: Bug-1581754_remove-grid-EdInsertTOC.patch

Apply after patch from Bug 1581558.

Comment 7

[Magnus Melin [:mkmelin]]

Comment on attachment 9093373 [details] [diff] [review]
Bug-1581754_remove-grid-EdInsertTOC.patch

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

::: mail/components/compose/content/dialogs/EdInsertTOC.xul
@@ +35,5 @@
> +            &tag.label;
> +          </html:th>
> +          <html:th>
> +            &class.label;
> +          </html:th>

for cases like this were it would fit very nicely on one line, maybe change it to

<html:th>&class.label;</html:th>

Comment 8

[Khushil Mistry [:khushil324]]

Attachment: Bug-1581754_remove-grid-EdInsertTOC.patch

Comment 9

[Pulsebot]

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