1489791 - [de-xbl] Remove calendar-caption binding.

Status: RESOLVED FIXED

(Calendar :: Lightning Only, enhancement)

Description

[Arshad Khan [:arshad]]

Opening binding and replacing it with xul elements.

Binding link - https://searchfox.org/comm-central/source/calendar/base/content/calendar-item-bindings.xml#14

Comment 1

[Arshad Khan [:arshad]]

Attachment: calendar-caption.patch

Comment 2

[Arshad Khan [:arshad]]

Attachment: calendar-caption.patch

Comment 3

[Magnus Melin [:mkmelin]]

Comment on attachment 9007480 [details] [diff] [review]
calendar-caption.patch

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

Since this is calendar/ someone from there should review

::: calendar/base/content/dialogs/calendar-event-dialog-reminder.xul
@@ +48,4 @@
>      </hbox>
>    </vbox>
>  
> +  <hbox class="calendar-caption" align="center" id="reminder-details-caption">

here and elsewhere, note that the id comes first, then align the attributes

::: calendar/base/content/dialogs/calendar-summary-dialog.xul
@@ +349,5 @@
>  
>    <!-- Description -->
>    <box id="item-description-box" hidden="true" orient="vertical" flex="1">
>      <spacer class="default-spacer"/>
> +    <hbox class="calendar-caption" align="center">

while you're here, for the hboxes you're adding, if they are missing an id you could go ahead and add one

Comment 4

[Arshad Khan [:arshad]]

Attachment: calendar-caption.patch

Comment 5

[Arshad Khan [:arshad]]

Comment on attachment 9008721 [details] [diff] [review]
calendar-caption.patch

hey check the id values of the calendar captions and also flag someone who is right person to review from calendars team.

Comment 6

[Magnus Melin [:mkmelin]]

Comment on attachment 9008721 [details] [diff] [review]
calendar-caption.patch

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

nit: in the commit message "Bug 1489791 - ", not "Bug 1489791: "

You can flag MakeMyDay or Philipp for review

::: calendar/base/content/dialogs/calendar-event-dialog-reminder.xul
@@ +104,5 @@
>  
> +  <hbox class="calendar-caption" id="reminder-actions-caption" align="center">
> +    <label value="&reminder.action.label;" control="reminder-actions-menulist"
> +      class="header"/>
> +    <separator class="groove" flex="1"/>

Please make id the first attribute

And align attributes beneath each other (at least when the line needs to wrap)

<label value="&reminder.action.label;" control="reminder-actions-menulist"
       class="header"/>

Goes in the other places too of course

Comment 7

[Arshad Khan [:arshad]]

Attachment: calendar-caption.patch

Comment 8

[[:MakeMyDay]]

Comment on attachment 9009040 [details] [diff] [review]
calendar-caption.patch

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

Thanks, looks good, but please fix the style of the commit message when updating the reviwer for checkin to what magnus mentioned in comment 6.

Comment 9

[Arshad Khan [:arshad]]

Attachment: calendar_caption_f.patch

Comment 10

[Jorg K (CEST = GMT+2)]

We really need a try run. I had two patches presented for landing so far that caused test failures:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=28496a4ad1127d65ad7a31c3a19db77b476137d9

Comment 11

[Jorg K (CEST = GMT+2)]

Try looks OK, the failure we see is pre-existing.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/d197d42b67d8
Remove calendar-caption binding. r=MakeMyDay