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 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 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 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?