Closed Bug 1581754 Opened 4 months ago Closed 4 months ago

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

Categories

(Thunderbird :: Message Compose Window, task)

task
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 71.0

People

(Reporter: khushil324, Assigned: khushil324)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 3 obsolete files)

No description provided.
Assignee: nobody → khushil324
Attachment #9093230 - Flags: review?(mkmelin+mozilla)
Status: NEW → ASSIGNED
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
Attachment #9093230 - Flags: review?(mkmelin+mozilla)

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

Attachment #9093230 - Attachment is obsolete: true
Attachment #9093256 - Flags: review?(mkmelin+mozilla)

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

Apply after patch from Bug 1581558.

Attachment #9093256 - Attachment is obsolete: true
Attachment #9093256 - Flags: review?(mkmelin+mozilla)
Attachment #9093373 - Flags: review?(mkmelin+mozilla)
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>
Attachment #9093373 - Flags: review?(mkmelin+mozilla) → review+
Attachment #9093373 - Attachment is obsolete: true
Attachment #9093895 - Flags: review+
Keywords: checkin-needed

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/59ea04a195f6
remove grid usage from EdInsertTOC.xul. r=mkmelin

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