The abstract dragndropContainer: https://searchfox.org/comm-central/rev/5a670c59f9004ef9be4874cfbfe57ec2ef3b260f/calendar/base/content/widgets/calendar-widgets.xml#251

Child classes:

• calendar-month-day-box - https://searchfox.org/comm-central/rev/5a670c59f9004ef9be4874cfbfe57ec2ef3b260f/calendar/base/content/calendar-month-view.xml#124
• https://searchfox.org/comm-central/rev/5a670c59f9004ef9be4874cfbfe57ec2ef3b260f/calendar/base/content/calendar-multiday-view.xml#1900

I need a little bit of help with this one.
Overall, I'm uploading the WIP patch to gather feedback regarding the approach, if the abstract class is correct and everything looks properly written.

Onto the problem I have.
The XBL version of this uses the <children/> markup, which automatically adds and removes child nodes in that location.
This used to work perfectly with the current structure, even if in the calendar box we have other child nodes, like the week and day label.

After converting this to CE, it doesn't work anymore as the child nodes (events) are appended in the wrong location, and when a date is set and all the child should be removed, also the week and day label are removed.

I tried to update the code in order to properly append the events inside the day-items container, but that causes many problems since every single event is tightly bounded to its parent node and its attributes.

I need some feedback regarding the proper approach to follow, since any sort of change to the way the events are appended to the calendar box will affect many files.

(In reply to Magnus Melin [:mkmelin] from comment #2)

You write "after converting this to CE, it doesn't work anymore as the child
nodes (events) are appended in the wrong location, and when a date is set
and all the child should be removed, also the week and day label are
removed. "

But why does that happen?

I know it seems weird, let me give you some visual examples and see if I can explain myself properly.

This is the current XBL structure of the calendar box: https://searchfox.org/comm-central/source/calendar/base/content/calendar-month-view.xml#125

As you can see inside the main container, we have an hbox to render the week and day label, then a scrollable area wrapping around the <children/> tag to accommodate all the events.

When the calendar is generated, or a new event is set, all the previous events in each box are removed: https://searchfox.org/comm-central/source/calendar/base/content/calendar-month-view.xml#210

Moreover, when a new item is added, the calendar expects the event to have the main day box container as its parent, in order to inherit the necessary attributes, even if the actual event ends up being appended in the <children/> tag location: https://searchfox.org/comm-central/source/calendar/base/content/calendar-month-view.xml#248

This approach doesn't work in a CE because:

• The removeChildren() method in a CE removes everything, including the other child nodes for the week and day label.
• The new items are appended after all the other child nodes, outside the supposed scrollable area that needs to accommodate them.
• Updating the code to remove and append the events inside the this.dayItems element, creates a cascading series of bugs because the entire calendar is wired to tap the events as direct children of the main day box container.

Therefore my question regarding what should I do. Should I go around the whole calendar and consequentially update all the methods interacting with events, or is there something obvious I'm missing and this is just a mess in my head?

for readability when there are quotes, you can use ` instaed of escaping,

I originally did that, but then I was having eslint flagging it as linting error as it expects double quotes for strings.
Do I ignore it?

I think I identified the core problem and found a solution.
The main issue I was having is the fact that the same abstract class is used by the header container and the single day box, which one has no child nodes while the other has a whole different structure with nested nodes.
I implemented a couple of extra conditions to handle the exceptions and everything seems to work.

I have a couple of failing mozmill tests due to the change of container.
Fixing those right now.

Done!

(In reply to Magnus Melin [:mkmelin] from comment #7)

I think it should be calendar-dnd-widgets.js (with an s.)

Updated!

Looks good, afaict. I've done some unbitrotting, so attaching a patch for
that next.

Thanks for doing that.

@@ +62,5 @@

  addDropShadows() {
      if (this.mDropShadows) {

you could change this to return early instead

Good call, done.

@@ +228,5 @@
MozElements.CalendarDnDContainer = CalendarDnDContainer;

why this?

A leftover from when I started working on the patch as I was expecting to be needing to extend this class on multiple other files.

Ready for a full review from Philipp.

I updated the patch as suggested.
After unbitrotting, also a lot of mozmill tests were failing. I fixed those.

I'm requesting a review from Magnus and Philipp again since some substantial things have changed.

Here's the try run: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=7b79b5c460c2a2cc39969836298731d493392a37

No, it doesn't you need to rename the file.

EDIT: Oops, spoke too early. Rename (or copy) can be used for a straight XML to JS translation. But here, three bindings are removed from three different XML files leaving some content in those files and a JS file is created instead. So there's no option to just creating the new file.

Alex, can you please submit your patches with 8 lines of context. Add this to your mercurial.ini:
[diff]
git = 1
showfunc = 1
unified = 8

At times I need to compare patches and they are totally different if they have different lines of context.

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/5ce69de5c6c6
[de-xbl] convert the dragndropContainer binding. r=mkmelin,philipp DONTBUILD