1568311 - remove grid usage from comm/calendar/base/content/dialogs/calendar-event-dialog-attendees.xul

Status: RESOLVED FIXED

(Thunderbird :: General, task)

Comment 1

[Khushil Mistry [:khushil324]]

Attachment: Bug-1568311_remove-grid-calendar-event-dialog-attendees.patch

Comment 2

[Magnus Melin [:mkmelin]]

Comment on attachment 9080144 [details] [diff] [review]
Bug-1568311_remove-grid-calendar-event-dialog-attendees.patch

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

I notice some funny resizing of the main right and left areas right after opening the dialog (they grow a bit, making the dialog itself grow)

::: calendar/base/content/dialogs/calendar-event-dialog-attendees.xul
@@ +239,5 @@
> +              <xul:label value="&event.freebusy.legend.free;"/>
> +            </td>
> +          </tr>
> +          <tr>
> +            <td style="display:inline-flex;">

seems like an odd thing to put on a td. should it (or something else) be in the .legend rule?

@@ +277,5 @@
>      <vbox>
> +      <table xmlns="http://www.w3.org/1999/xhtml">
> +        <tr>
> +          <td>
> +            <xul:spacer/>

I assume we can remove this?

Comment 3

[Khushil Mistry [:khushil324]]

Attachment: Bug-1568311_remove-grid-calendar-event-dialog-attendees.patch

Comment 4

[Magnus Melin [:mkmelin]]

Comment on attachment 9083787 [details] [diff] [review]
Bug-1568311_remove-grid-calendar-event-dialog-attendees.patch

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

::: calendar/base/content/dialogs/calendar-event-dialog-attendees.xul
@@ +162,5 @@
> +            <td>
> +              <xul:image class="role-icon" role="REQ-PARTICIPANT"/>
> +            </td>
> +            <td>
> +              <xul:label value="&event.attendee.role.required;"/>

No need for label here. Just plain &event.attendee.role.required; as the td content

same for the other cases (that are just text)

Comment 5

[Khushil Mistry [:khushil324]]

Attachment: Bug-1568311_remove-grid-calendar-event-dialog-attendees-xul.patch

Comment 6

[Magnus Melin [:mkmelin]]

Comment on attachment 9084614 [details] [diff] [review]
Bug-1568311_remove-grid-calendar-event-dialog-attendees-xul.patch

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

::: calendar/base/content/dialogs/calendar-event-dialog-attendees.xul
@@ +250,5 @@
> +            <td>
> +              <xul:box class="legend" status="BUSY"/>
> +            </td>
> +            <td>
> +              &event.freebusy.legend.busy;"

there's a stray " after this showing in the text

@@ +285,5 @@
> +        </tr>
> +        <tr>
> +          <td>
> +            <xul:label value="&newevent.from.label;" control="event-starttime"/>
> +          </td>

I think From and To should be <th>. Questionable if the label should have the control= , since it's pretty clear (then) that this row is for From/To, and there are also other things (like timezone) that it's connected to.

I would just have them as
<th>&newevent.from.label;</th>

Comment 7

[Khushil Mistry [:khushil324]]

Attachment: Bug-1568311_remove-grid-calendar-event-dialog-attendees-xul.patch

Comment 8

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 9084709 [details] [diff] [review]
Bug-1568311_remove-grid-calendar-event-dialog-attendees-xul.patch

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

::: calendar/base/content/dialogs/calendar-event-dialog-attendees.xul
@@ +259,5 @@
> +              <xul:box class="legend" status="BUSY_UNAVAILABLE"/>
> +            </td>
> +            <td>
> +              &event.freebusy.legend.busy_unavailable;
> +            </td>

I think we can condense this a bit by putting td's in line with entities. Possibly also the boxes if there is only one?

Comment 9

[Khushil Mistry [:khushil324]]

Attachment: Bug-1568311_remove-grid_calendar-event-dialog-attendees-xul.patch

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=3bd44543601afcb62ccdab6192fe3c81ee4c2cc3

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/875f646d102e
remove grid usage from calendar-event-dialog-attendees.xul. r=philipp