Closed Bug 1489791 Opened Last year Closed Last year

[de-xbl] Remove calendar-caption binding.

Categories

(Calendar :: Lightning Only, enhancement)

enhancement
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: arshad, Assigned: arshad)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 4 obsolete files)

Opening binding and replacing it with xul elements.

Binding link - https://searchfox.org/comm-central/source/calendar/base/content/calendar-item-bindings.xml#14
Assignee: nobody → arshdkhn1
Status: NEW → ASSIGNED
Attached patch calendar-caption.patch (obsolete) — Splinter Review
Attached patch calendar-caption.patch (obsolete) — Splinter Review
Attachment #9007479 - Attachment is obsolete: true
Attachment #9007480 - Flags: review?(mkmelin+mozilla)
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
Attachment #9007480 - Flags: review?(mkmelin+mozilla) → feedback+
Attached patch calendar-caption.patch (obsolete) — Splinter Review
Attachment #9007480 - Attachment is obsolete: true
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.
Attachment #9008721 - Flags: feedback?(mkmelin+mozilla)
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
Attachment #9008721 - Flags: feedback?(mkmelin+mozilla)
Attached patch calendar-caption.patch (obsolete) — Splinter Review
Attachment #9008721 - Attachment is obsolete: true
Attachment #9009040 - Flags: review?(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.
Attachment #9009040 - Flags: review?(makemyday) → review+
Attachment #9009040 - Attachment is obsolete: true
Attachment #9009434 - Flags: review+
Keywords: checkin-needed
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
Try looks OK, the failure we see is pre-existing.
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/d197d42b67d8
Remove calendar-caption binding. r=MakeMyDay
Status: ASSIGNED → RESOLVED
Closed: Last year
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 6.6
You need to log in before you can comment on or make changes to this bug.