Closed
Bug 1554648
Opened 5 years ago
Closed 5 years ago
[de-xbl] convert the calendar-subscriptions-richlistitem binding to <richlistitem is="calendar-subscriptions-richlistitem">
Categories
(Calendar :: General, task)
Calendar
General
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 69.0
People
(Reporter: mkmelin, Assigned: khushil324)
References
Details
Attachments
(3 files, 1 obsolete file)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
image/png
|
Details | |
(deleted),
patch
|
khushil324
:
review+
|
Details | Diff | Splinter Review |
Convert the calendar-subscriptions-richlistitem binding to <richlistitem is="calendar-subscriptions-richlistitem">
Assignee | ||
Comment 1•5 years ago
|
||
Attachment #9069915 -
Flags: review?(philipp)
Attachment #9069915 -
Flags: feedback?(mkmelin+mozilla)
Assignee | ||
Comment 2•5 years ago
|
||
This patch will be applied before the main patch for testing purpose. Then run openCalendarSubscriptionsDialog() in the console to open the dialogue.
Assignee | ||
Comment 3•5 years ago
|
||
Assignee | ||
Updated•5 years ago
|
Status: NEW → ASSIGNED
Reporter | ||
Comment 4•5 years ago
|
||
Comment on attachment 9069915 [details] [diff] [review] Bug-1554648_de-xbl_calendar-subscriptions-richlistitem.patch Review of attachment 9069915 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. ::: calendar/base/content/widgets/calendar-subscriptions-list.js @@ +70,4 @@ > return true; > } else { > return false; > } can we just do return checkbox.getAttribute("checked") == "true"; @@ +88,5 @@ > + if (checkbox.getAttribute("disabled") == "true") { > + return true; > + } else { > + return false; > + } just return checkbox.getAttribute("disabled") == "true"; @@ +99,5 @@ > + } > + } > + > + customElements.define("calendar-subscriptions-richlistitem", MozCalendarSubscriptionsRichlistitem, > + { "extends": "richlistitem" }); please no quotes around extends
Attachment #9069915 -
Flags: feedback?(mkmelin+mozilla) → feedback+
Comment 5•5 years ago
|
||
Comment on attachment 9069915 [details] [diff] [review] Bug-1554648_de-xbl_calendar-subscriptions-richlistitem.patch Review of attachment 9069915 [details] [diff] [review]: ----------------------------------------------------------------- r=me with Magnus' comments considered. ::: calendar/base/content/widgets/calendar-subscriptions-list.js @@ +24,2 @@ > > + this.setAttribute("is", "calendar-subscriptions-richlistitem"); If the connectedCallback of this class is called, isn't it already a `calendar-subscriptions-richlistitem`? Do we need this setAttribute call?
Attachment #9069915 -
Flags: review?(philipp) → review+
Reporter | ||
Comment 6•5 years ago
|
||
Element's created through script do not automatically get the "is" attribute, so it's good practice so set it. (E.g. couldn't match css on them reliably otherwise.)
Assignee | ||
Comment 7•5 years ago
|
||
Attachment #9069915 -
Attachment is obsolete: true
Attachment #9070826 -
Flags: review+
Assignee | ||
Updated•5 years ago
|
Keywords: checkin-needed
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/a7bf84ab2a75
[de-xbl] convert the calendar-subscriptions-richlistitem binding to custom element. r=philipp
Updated•5 years ago
|
Target Milestone: --- → 7.1
You need to log in
before you can comment on or make changes to this bug.
Description
•