[de-xbl] convert the calendar-editable-item binding and derivatives calendar-month-day-box-item + calendar-event-box
Categories
(Calendar :: General, task)
Tracking
(Not tracked)
People
(Reporter: mkmelin, Assigned: khushil324)
References
Details
Attachments
(1 file, 5 obsolete files)
89.84 KB,
patch
|
pmorris
:
review+
|
Details | Diff | Splinter Review |
calendar-editable-item: https://searchfox.org/comm-central/rev/5a670c59f9004ef9be4874cfbfe57ec2ef3b260f/calendar/base/content/calendar-view-core.xml#16
Child classes:
- calendar-month-day-box-item - https://searchfox.org/comm-central/rev/5a670c59f9004ef9be4874cfbfe57ec2ef3b260f/calendar/base/content/calendar-month-view.xml#16
- calendar-event-box - https://searchfox.org/comm-central/rev/5a670c59f9004ef9be4874cfbfe57ec2ef3b260f/calendar/base/content/calendar-multiday-view.xml#2036
Reporter | ||
Updated•6 years ago
|
Assignee | ||
Comment 2•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=22f422a094a3e6e02d6b2c9ec639ebf1635c50e2
I have updated the patch with corrections on test failures.
Assignee | ||
Updated•6 years ago
|
Reporter | ||
Comment 3•6 years ago
|
||
Assignee | ||
Comment 4•6 years ago
|
||
Reporter | ||
Comment 5•6 years ago
|
||
Assignee | ||
Comment 6•6 years ago
|
||
(In reply to Magnus Melin [:mkmelin] from comment #5)
when is dragOccurrence (item) null?
When calendar-muldiday-view i.e. Week and Day views, is not focused and you try to create an event with click and drag. And that call in trace will create problem for this.mSelectedItems.
Reporter | ||
Comment 7•6 years ago
|
||
But it wasn't a problem in the xbl code, so why is it a problem now? That sounds like something is wrong.
Assignee | ||
Comment 8•6 years ago
•
|
||
(In reply to Magnus Melin [:mkmelin] from comment #7)
But it wasn't a problem in the xbl code, so why is it a problem now? That sounds like something is wrong.
I found this issue on Trunk. So tried to solve it with this patch.
Calendar Views involves lots of calls of connectedCallback. In XBL, when we constructor anything, child elements append at the time of construction. Where in CE, connectedCallback runs when you append it to another element. Generally, we append child elements of CE during the connectedCallbacks. I find many similar problems which I needed to consider in this conversion so I guess, this can be a problem and it has created such a situation. We can look into it after converting this element as this will be the last CE in this view.
Reporter | ||
Comment 9•6 years ago
|
||
Assignee | ||
Comment 10•6 years ago
|
||
Reporter | ||
Comment 11•6 years ago
|
||
Assignee | ||
Comment 12•6 years ago
|
||
Reporter | ||
Comment 13•6 years ago
|
||
Comment 14•6 years ago
|
||
Assignee | ||
Comment 15•6 years ago
|
||
(In reply to Paul Morris [:pmorris] from comment #14)
Optional suggestion: instead of duplicating code in the body of the mouseout
and mousemove handlers, put it into its own function and call it. Something
likethis.startItemDrag
. Just a suggestion, I don't insist on it.
Nice suggestion.
I think we should put this CE in its own separate file
("calendar-editable-item.js"), rather than adding it to this file. Better
to keep this file just for the 4 main view CEs (which are part of a
non-trivial class hierarchy that's good to keep separate from other things).It's tempting to think about moving the other CEs (calendar-event-box and
calendar-month-day-box-item) out into their own files, so it's one CE per
file, with file names corresponding to the CE name. But lets do that in a
follow-up if we do.
Agree.
It's odd that there are two 'dragstart' event listeners, can we combine them?
One is Bubbling phase and another is capturing phase. So I have kept it two event listeners.
Assignee | ||
Comment 16•6 years ago
|
||
Comment 17•6 years ago
|
||
Assignee | ||
Comment 18•6 years ago
|
||
Comment 19•6 years ago
|
||
Assignee | ||
Comment 20•6 years ago
|
||
Assignee | ||
Updated•6 years ago
|
Comment 21•6 years ago
|
||
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/40db4b5c31ff
[de-xbl] convert calendar-editable-item binding and derivatives to custom elements. r=pmorris
Comment 22•6 years ago
|
||
Down to the last few bindings left!
With these three gone, Calendar looks XBL-free. Left are:
chat/content/imtooltip.xml
13 <binding id="tooltip" extends="chrome://global/content/bindings/popup.xml#panel">
mail/base/content/search.xml
34 <binding id="glodaSearch"
Bug 1506529 and bug 1542720. I believe there's also still work in bug 1565075 (waiting for bug 1534455) and hopefully fixing bug 1569492.
Description
•