Closed
Bug 437412
Opened 16 years ago
Closed 16 years ago
zero-length and short events are shown at the wrong times in the timegrid
Categories
(Calendar :: Calendar Frontend, defect)
Calendar
Calendar Frontend
Tracking
(Not tracked)
VERIFIED
FIXED
0.9
People
(Reporter: andreas.treumann, Assigned: berend.cornelius09)
References
Details
(Keywords: regression)
Attachments
(6 files, 5 obsolete files)
23.90 KB,
patch
|
Fallen
:
review+
|
Details | Diff | Splinter Review |
52.07 KB,
image/jpeg
|
Details | |
17.67 KB,
image/png
|
Details | |
26.17 KB,
image/jpeg
|
Details | |
82.54 KB,
image/png
|
Details | |
17.26 KB,
patch
|
berend.cornelius09
:
review+
dbo
:
ui-review+
|
Details | Diff | Splinter Review |
STEPS TO REPRODUCE: =================== - create an event start time 8.00 AM , end time 8:00 AM - create an second event start time 8.30 AM , end time 8:30 AM - check the calendar views -> looks okay - create an third event star time 8:00 AM, end time 9:00 AM - check the calendar view (dayview/weekview) RESULT: ======= - the second event jumps to a wrong place in the time grid EXPECTED RESULT: ================ - events should be displayed correctly REPRODUCIBLE: ============= - always
Updated•16 years ago
|
Summary: zero-lenght events are sometimes not in sync with the timegrid → zero-length and short events are sometimes not in sync with the timegrid
Updated•16 years ago
|
Flags: blocking-calendar0.9?
Updated•16 years ago
|
Flags: blocking-calendar0.9? → blocking-calendar0.9+
Assignee | ||
Updated•16 years ago
|
Assignee: nobody → Berend.Cornelius
Updated•16 years ago
|
Blocks: 387402
Keywords: regression
Summary: zero-length and short events are sometimes not in sync with the timegrid → zero-length and short events are shown at the wrong times in the timegrid
Assignee | ||
Comment 2•16 years ago
|
||
This patch basically solves the problem and hopefully all other issues around overlapping event boxes. However I noticed one serious side effect that should not be ignored: When the mouse hovers over an event with small width the grippys are shown (as usually) and the event-box starts to change its width for the time the mouse pointer resides over the box (not as usually). This should not be but I am ashamed to say that I just could not figure out how to resolve this problem. Philipp - (or anybody else), could you have a look at this? I am aware that it's formal wise not correct to ask for a review already at this state, but I am hoping that there is just another uncritical thing to add to this patch.
Attachment #331732 -
Flags: review?(philipp)
Updated•16 years ago
|
Status: NEW → ASSIGNED
Comment 3•16 years ago
|
||
Comment on attachment 331732 [details] [diff] [review] patch v. #1 >+ var configBox = document.getAnonymousElementByAttribute(this, "anonid", "config-box"); >+ var minSize = configBox.getOptimalMinSize(); >+// configBox.setAttribute("orient", orient); >+// var minSize = 25; //configBox.getOptimalMinSize(); >+ this.mMinDuration = Components.classes["@mozilla.org/calendar/duration;1"] >+ .createInstance(Components.interfaces.calIDuration); >+ this.mMinDuration.minutes = parseInt(minSize/this.mPixPerMin); Either remove or annotate commented code. >- var chunkBox = createXULElement("calendar-event-box"); >+ var chunkBox = configBox.cloneNode(false); >+// var chunkBox = createXULElement("calendar-event-box"); I'm not a fan of cloning nodes, but in any case remove the commented code >+ >+ >+function getOtherOrientation(aOrientation) { >+ if (aOrientation != null) { >+ if (aOrientation == "vertical") { >+ return "horizontal"; >+ } else { >+ return "vertical"; >+ } >+ } else { >+ return "vertical"; >+ } >+} It should be enough to do return (aOrientation == "vertical" ? "horizontal" : "vertical"); Also, redundant newlines above. r=philipp
Attachment #331732 -
Flags: review?(philipp) → review+
Assignee | ||
Comment 4•16 years ago
|
||
Andreas tested the last patch this morning and found one bug that I fixed with this patch. I also addressed Philipp's comments in the new patch. Still the layout problem remains. As far as I see this patch fixes all issues regarding overlapping items. If Philipp does not succeed to solve the layout issue it is probably an option to remove the rules for the grippy images from the css file temporarily.
Attachment #331732 -
Attachment is obsolete: true
Assignee | ||
Updated•16 years ago
|
Attachment #331842 -
Flags: review?(philipp)
Reporter | ||
Comment 5•16 years ago
|
||
Reporter | ||
Comment 6•16 years ago
|
||
Checked patch v. #2: - no drag&drop shadow in the day view - resizing the horizontal width of the calendar view -> some boxes overlap and don't use the complete horizontal space (see screen shot)
Reporter | ||
Comment 7•16 years ago
|
||
The first point is wrong. The d&d shadow is visible, but a very long time is needed to get the shadow.
Reporter | ||
Comment 8•16 years ago
|
||
I used a second PC and it's not possible for me to reproduce the performance issue. Only the screen shot issue is valid!
Assignee | ||
Comment 9•16 years ago
|
||
Concerning the performance I still see room for improvement as "getlayoutEnd" is currently called various times in a loop and instead "layoutEnd" could be kept in a member variable. I tried the recent days to resolve the remaining problem with the dancing events on mouseover but I was not successfull. Maybe it's an option to remove the rules calendar-event-box[orient="vertical"]:hover .calendar-event-box-grippy-top { list-style-image: url("chrome://calendar/skin/event-grippy-top.png"); } calendar-event-box[orient="vertical"]:hover .calendar-event-box-grippy-bottom { list-style-image: url("chrome://calendar/skin/event-grippy-bottom.png"); } calendar-event-box[orient="horizontal"]:hover .calendar-event-box-grippy-top { list-style-image: url("chrome://calendar/skin/event-grippy-left.png"); } calendar-event-box[orient="horizontal"]:hover .calendar-event-box-grippy-bottom { list-style-image: url("chrome://calendar/skin/event-grippy-right.png"); } temporarily until we find a solution.
Comment 10•16 years ago
|
||
Comment on attachment 331842 [details] [diff] [review] [checked in] patch v. #2 > var chunkBox = createXULElement("calendar-event-box"); >+// var chunkBox = createXULElement("calendar-event-box"); Remove comment r=philipp
Attachment #331842 -
Flags: review?(philipp) → review+
Updated•16 years ago
|
Attachment #331842 -
Attachment description: patch v. #2 → [checked in] patch v. #2
Comment 11•16 years ago
|
||
Patch checked in, keeping open to fix the hover problem.
Assignee: Berend.Cornelius → philipp
Status: ASSIGNED → NEW
Comment 12•16 years ago
|
||
Day and week view are empty in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.2pre) Gecko/2008080918 Calendar/0.6a1. Error Console shows: Error: syntax error Source File: chrome://calendar/content/calendar-multiday-view.xml Line: 1111, Column: 14 Source Code: } Error: aDayBox.startLayoutBatchChange is not a function Source File: chrome://calendar/content/calendar-multiday-view.xml Line: 3242
Comment 13•16 years ago
|
||
I see also this error. With removing this bracket at line 1111, the Day and Week views works again.
Updated•16 years ago
|
Whiteboard: [needs patch]
Comment 14•16 years ago
|
||
Checked in a bustage-fix type patch to remove the bracket. Thanks for testing.
Comment 15•16 years ago
|
||
Although the remaining issue is a bug, I'd almost call it a feature ;-) Its kind of nice that when you hover over an event you get to see more of it. Christian what do you think?
Comment 16•16 years ago
|
||
I'd prefer a different "solution". This "feature" is from an ux perspective more or less a bug. The jumping events are not nice, even if they are easer to select. We should turn back this change. Is the overlapping event caused by the change? See attachment.
Comment 17•16 years ago
|
||
Reporter | ||
Comment 18•16 years ago
|
||
Comment 19•16 years ago
|
||
To be honest, the only difference I see is that the boxes of the old build seem to be wider (you can read more of the text).
Assignee | ||
Comment 20•16 years ago
|
||
In the screenshot of commment #18 can be seen that actually the eventbox in the bottom right corner has the right width (2/3 of the width of the column). The other three eventboxes are supposed to have equal width (each 1/3 of the column width) but they actually differ. The first one with the category consumes more space than it is actually allowed to have. All these 3 boxes are on one layer within a stack element whereas the fourth eventbox is wihin another layer. Within each layer open space is filled with xul:spacer elements. So what I tried to do (among many other things) is to set "equalsize" to the parental box of the three eventboxes to enforce equal width - but without success and I don't know why. I admit that I ran out of ideas on how to solve this issue and I am thankful for any assistance. I could not reproduce the bug scenario with eventboxes without category. Unfortunately the behaviour is not any better on trunk as I tested so we somehow won't get around to deal with this problem.
Assignee | ||
Comment 21•16 years ago
|
||
This patch should resolve the problem with the dancing events on mouse hover. By setting the "visibility" attribute instead of "display" I take care the grippies don't lose their size when made invisible. Yet I still don't have a solution for the wider category-eventboxes.
Assignee: philipp → Berend.Cornelius
Status: NEW → ASSIGNED
Attachment #334761 -
Flags: review?(philipp)
Assignee | ||
Comment 22•16 years ago
|
||
I think I found a solution for the eventboxes containing categories, too and extended the patch v. #3 by this functionality: I moved the category child element into the stack element of the event box where actually all other anonymous elements also reside. One side effect is that now the title is also using the space of the category box which I find very positive although admittedly the category box has another background than the rest of the event-box. We currently use the space of the category box for nothing else than the display of the category color and I have always found this is a waste of space.
Attachment #334761 -
Attachment is obsolete: true
Attachment #334777 -
Flags: ui-review?(daniel.boelzle)
Attachment #334777 -
Flags: review?(philipp)
Attachment #334761 -
Flags: review?(philipp)
Assignee | ||
Comment 23•16 years ago
|
||
As can be seen in this screenshot the title of the category-event now also uses the space of the category-box.
Comment 24•16 years ago
|
||
Comment on attachment 334777 [details] [diff] [review] patch v. #4 The alarm bell overlays on the category space which IMO looks odd. I don't feel like the right person to judge on this.
Attachment #334777 -
Flags: ui-review?(daniel.boelzle)
Attachment #334777 -
Flags: ui-review-
Attachment #334777 -
Flags: review?(philipp)
Attachment #334777 -
Flags: review?(christian.jansen)
Comment 25•16 years ago
|
||
Comment on attachment 334777 [details] [diff] [review] patch v. #4 Oops, I should get some sleep.
Attachment #334777 -
Flags: ui-review?(christian.jansen)
Attachment #334777 -
Flags: ui-review-
Attachment #334777 -
Flags: review?(philipp)
Attachment #334777 -
Flags: review?(christian.jansen)
Assignee | ||
Comment 26•16 years ago
|
||
I tried to address Daniel's concerns and put the alarm-image left to the category-box. To my dismay I observed that the problem with the colliding events returned. As I found out the culprit is the alarm image that consumes the space that it needs similarily to the category-box beforehand. The patch attached does not solve the problem but merely contains the current state of the art and thus contains some codelines that still need a clean up. I still keep asking for assistance! I talked to Daniel about it: If we don't come with a proper solution I will and extend the alarm.pngs to the right by the width of the category-box with a transparent background. I know that this is an ugly hack of course that should only be temporary.
Assignee | ||
Comment 27•16 years ago
|
||
>If we don't come with a proper solution I will and
>extend the alarm.pngs to the right by the width of the category-box with a
>transparent background.
This is not a solution because the alarm images do not always turn up in combination with a category.
Comment 28•16 years ago
|
||
Berend, are you uploading a new version of this patch today? If you have any further problems, feel free to give me a call.
Updated•16 years ago
|
Attachment #334777 -
Flags: review?(philipp)
Assignee | ||
Comment 29•16 years ago
|
||
I added the functionality as desired and hope I got it all straight now. I have to send special thanks to philipp who help to spot the reason why the subelements of the eventboxes consumed too much space (It was the width of the gradient that for some reason was set to 1px).
Attachment #334898 -
Attachment is obsolete: true
Attachment #335097 -
Flags: ui-review?(daniel.boelzle)
Attachment #335097 -
Flags: review?(philipp)
Assignee | ||
Comment 30•16 years ago
|
||
As I investigated the source code in question that caused the problem was a deprecated relict and was checked in by Bug 402683 – new category layout causes colliding event boxes
Comment 31•16 years ago
|
||
Comment on attachment 335097 [details] [diff] [review] patch v. #6 This patch makes the gripbar be larger than it is thought to be. It looks a bit stretched.
Attachment #335097 -
Flags: ui-review?(daniel.boelzle)
Attachment #335097 -
Flags: review?(philipp)
Attachment #335097 -
Flags: review-
Comment 32•16 years ago
|
||
This patch takes care, the only thing I really changed is adding xbl:inherts="align=whichside" to the grippy's "thebox". I also took the liberty of removing crufty code from the gripbar. berend, if this version is r+ for you, please go ahead and check it in.
Attachment #334777 -
Attachment is obsolete: true
Attachment #335097 -
Attachment is obsolete: true
Attachment #335167 -
Flags: review?(Berend.Cornelius)
Attachment #334777 -
Flags: ui-review?(christian.jansen)
Updated•16 years ago
|
Whiteboard: [needs patch] → [needs review]
Assignee | ||
Comment 33•16 years ago
|
||
Comment on attachment 335167 [details] [diff] [review] patch v. #7 patch looks good; r=berend
Attachment #335167 -
Flags: review?(Berend.Cornelius) → review+
Assignee | ||
Updated•16 years ago
|
Attachment #335167 -
Flags: ui-review?(daniel.boelzle)
Updated•16 years ago
|
Whiteboard: [needs review]
Updated•16 years ago
|
Attachment #335167 -
Flags: ui-review?(daniel.boelzle) → ui-review+
Assignee | ||
Comment 34•16 years ago
|
||
Thanks Philipp for you latest changes. I checked in the patch on trunk and MOZILLA_1_8_BRANCH. <I also took the liberty of removing crufty code from the gripbar. berend, if <this version is r+ for you, please go ahead and check it in. I also found the code quite hard to understand and think we should consolidate this after the next release has landed. It also makes sense to split up the file ./base/content/calendar-multiday-view in several other files. bug is fixed.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Updated•16 years ago
|
Target Milestone: --- → 0.9
Reporter | ||
Comment 35•16 years ago
|
||
Checked in lightning 2008082620 and Sunbird 20080826 -> VERIFIED.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•