460030 - Get rid of resize handler, use flex to draw view columns

Status: RESOLVED FIXED

(Calendar :: Calendar Frontend, defect)

Description

[Andreas Treumann]

STEPS TO REPRODUCE:
===================

- create a couple of events, one have to be an invitation (see screenshot)
- use the today pane to resize the day column in the day/weekview

RESULT:
=======

- the invitation event box overlap the normal events  

EXPECTED RESULT:
================

- the invitation event box should not overlap the normal events

REPRODUCIBLE:
=============

- always

Comment 1

[Andreas Treumann]

Attachment: overlap invitation

Comment 2

[Berend Cornelius]

Attachment: patch v. #1

In this patch I modified the code in such a way that no dotted borders (¨invitation needs action¨), alarm icons or category boxes are displayed and it improved the situation significantly. Of course this is not supposed to be a solution - I just wanted to know the actual root cause of this failure.

Comment 3

[Berend Cornelius]

Attachment: patch v. #2

The patch attached improves the situation by applying ¨collapsed¨ attributes instead of ¨hidden¨ to the category boxes and the alarm-icon, but does not really solve the problem.

Comment 4

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Berend, what is the status on this bug. Can you fix this bug for us?

I think its possible to solve this by adding |border: 2px solid transparent| to non-invitations, this way we have the same type of border and the calculations should work out.

Another potential solution is to use |outline| instead of |border| ?

Comment 5

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 353445 [details] [diff] [review]
patch v. #2

Berend, it seems you have already taken care of the files you are adding here. Are all bits from this patch already in the tree? If not, could you create a new patch? It doesn't have to fix the issue, just improve the situation.

Fixing the issue is an added bonus, but I'd like to progress on this bug.

Comment 6

[Markus Adrario [:Taraman]]

Attachment: Overlapping Event

Tried this on TB3B2 with yesterdays nightly of Lightning.

The overlap only appears for a moment when starting to resize and snaps to the right condition when you stop the mouse-movement (you don't have to release the button)
It also "works" with only normal events involved (see attached screenshot).
It seems to happen, when events with different widths above each other are involved. The event(s) spanning more than one "column" are then not resized correctly.
I tried Philipps Idea from comment #4 with a transparent border by doing the following in calendar-views.css @Line 597:
calendar-event-box,
calendar-editable-item,
calendar-month-day-box-item {
+    border: 2px solid transparent;
    opacity: 0.99;
}

No help, only the boxes moved a little further away from each other.

Comment 7

[Markus Adrario [:Taraman]]

I debugged a little more and found out that the "onResize" function is called only after the correct resizing took place again.

So I think the fault comes from some core-code that is responsible for the initial resizing and repositioning.

So maybe this is a bug in the core-system?

Comment 8

[Bernd]

I have seen your request on dev-layout,

There is no other way than to create a reproducible test case that can be attached to a bug. This is step 1, the digging where it happens is step 2.

Comment 9

[Markus Adrario [:Taraman]]

Digging further:
I'm narrowing it down to the "relayout" function ind Multiday-View [1].
It seems that the setting of the needed style for a box spanning multiple columns is computet correctly in the resizeHandler [2].
The problem is, that the function itself gets called too late. It happens when the resizing is done or you stop resizing-movement for a moment.

As of now I have now hint, why this happens.

to reproduce:
1. create a day-view setup like in [3].
2. Put a debugger; statement between the following 2 lines in calendar-multiday-view.xml:
        777   this.handleEvent = function(aEvent) {
        778       var resizeStyle = "min-width: 1px; min-height: 1px;";
3. start Venkman
4. resize the view and see when the debugger stops.


[1]:
http://mxr.mozilla.org/comm-central/source/calendar/base/content/calendar-multiday-view.xml#648

[2]:
http://mxr.mozilla.org/comm-central/source/calendar/base/content/calendar-multiday-view.xml#776

[3]:
https://bugzilla.mozilla.org/attachment.cgi?id=375024

Comment 10

[Markus Adrario [:Taraman]]

Unfortunately the above describerd is normal behaviour of the resize event.

As [1] states:
Notes:
The resize event is fired AFTER the window has been resized. 

I will try to use the overflow/underflow events [2] to get around this problem.

[1]: window.onresize
https://developer.mozilla.org/En/DOM/Window.onresize

[2]:
https://developer.mozilla.org/En/XUL/Events

Comment 11

[Markus Adrario [:Taraman]]

I found out, that the overflow/underflow events would indeed let the view update quicker.

But to fire these events we would have to add some element, which gets an overflow/underflow each time the view is resized.
I could not figure out how to do this, so I return this to unassigned.

Maybe we should think about leaving it as it is, since it only is really annoying, when you resize really fast. Else the onResize gets called often enough to get a decent outcome.

Comment 12

[Matthew Mecca [:mmecca]]

I'm unable to reproduce the issue with invitations using the original STR on the latest lightning nightly, but can confirm the odd resizing for all events spanning multiple columns during a resize, as mentioned in comment #6. Also, when the view is rotated and then resized using the unifinder, the multi-column events remain sized improperly after the resize is complete.

Comment 13

[Matthew Mecca [:mmecca]]

Attachment: [after beta] Proposed Patch

Sizes multi-span columns by using the flex property rather than setting the column width directly. This results in a smoother update while dragging during a resize, as well as eliminating the need for the resize event listeners.

Comment 14

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 400310 [details] [diff] [review]
[after beta] Proposed Patch


Code nits:
>+              if (layer.every(function (element) { return !element.specialSpan; })) {
>+                  columnCount = layer.length;
>+              }
The new js allows a syntax like:
if (layer.every(function (e) !e.specialSpan))

>+              }
>+              
>+              // add last empty column if necessary
Extra spaces in the middle line

Comment 15

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

This would surely simplify things, it does need some elaborate testing though. Back in the days of mozilla 1.8, when the war on boxes had its darkest time, Mickey found out that using the flex attribute caused problems with many many boxes at once. This needs testing, maybe with a calendar that has a lot of events in parallel and in the same hour. Archaeopteryx has a generated calendar with the german TV program, this might be a good start.

I'd really like to postpone this after the beta since the views are a quite central feature. Maybe we can have some sort of mozmill test, or maybe even a reftest that takes a picture of the views with the aforementioned calendar. For this we probably need a stable sorting mechanism though.

Comment 16

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Attachment: Screenshot - Problems with Proposed Patch

Just a few problems that show up with this patch. Mostly its just extra space, but in some cases things overlap. I haven't been able to try invitations though.

Comment 17

[Matthew Mecca [:mmecca]]

Attachment: [medium,test] Patch v2

Fixes spaces/overlaps.

Comment 18

[Michael Duelli]

Does this also solve bug 470425?

Comment 19

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Attachment: Problems with Invitations

Patch v2 works very good, I couldn't create any overlap errors with normal events. I did however produce some with invitations. To reproduce, create an ics file with two events very close to each other. Add the file to lightning, then add lines similar to the following:


ORGANIZER;CN=Philipp Kewisch;PARTSTAT=DECLINED:mailto:anyemailaddress@blab
 la.de 
ATTENDEE;RSVP=TRUE;PARTSTAT=NEEDS-ACTION;ROLE=REQ-PARTICIPANT:mailto:other
 [e-mail]


[e-mail] must be the email address set in the calendar properties dialog.

This causes the event to have the dotted border. I think you should easily be able to fix this by putting the border on the inside, i.e using "outline" instead of border. I assume this happens without the patch too though, so I'm fine with doing that in a separate bug.

This patch needs to wait until the next nightly builds, since I've already checked in a view-related patch today. I'll check v2 in tomorrow, then file the bug. I'll assign that bug to you, if you don't want to fix that issue go ahead and reassign to nobody.

Comment 20

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 416309 [details] [diff] [review]
[medium,test] Patch v2

>+          
>+          var columnCount = 1;
>+          var spanTotal = 0;

Change var to let, go ahead and do so for the surrounding var's too.


>+                  let lastStyle = "min-width: 1px; min-height: 1px; " + 
>+                                  "width: " + (columnCount - spanTotal) + "px; " + 
>+                                  "height: " + (columnCount - spanTotal) + "px;";
>+                  lastColumn.setAttribute("style", lastStyle);
This overwrites any other style attributes set, I'd suggest to assign style via js, i.e:

lastColumn.style.minWidth = "1px";
lastColumn.style.minHeight = "1px";
lastColumn.style.width =  (columnCount - spanTotal) + "px";
lastColumn.style.height = (columnCount - spanTotal) + "px";


This patch has r+ with the above fixed, leaving it on r? until tomorrow so I don't forget about it.

Comment 21

[Matthew Mecca [:mmecca]]

Attachment: Patch v3

Reviewed patch with requested fixes

Comment 22

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 428110 [details] [diff] [review]
Patch v3

r=philipp

Comment 23

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/bb36fc49a06f>

-> FIXED