368982 - visual enhancements for calendar event boxes

Status: VERIFIED FIXED

(Calendar :: Calendar Frontend, enhancement)

Description

[Michael Büttner (no reviews TFN)]

currently the event boxes are nothing more but filled rectangles, which doesn't look visually pleasant. this bug is dedicated to address this issue.

Comment 1

[Michael Büttner (no reviews TFN)]

Attachment: week-view screenshot

Comment 2

[Michael Büttner (no reviews TFN)]

Attachment: month-view screenshot

Comment 3

[Michael Büttner (no reviews TFN)]

Attachment: binary files

This archive contains the binary files for the patch. The images should be located at /calendar/resources/skin/classic - the archive already contains the images including this path. The following is a breakdown of the individual files.

gradient-overlay.png - used to modulate the main area of the event boxes
shadow-*.png - these files are used to add the drop-shadow effect to the event boxes

all of these images contain an alpha-channel in order to achieve the blending effect.

Comment 4

[Michael Büttner (no reviews TFN)]

Attachment: patch v1

and finally - the patch itself ;-)

basically this patch modifies the calendar-month-day-box-item, calendar-editable-item and calendar-event-box bindings in order to decorate them with the gradient and shadows.

Comment 5

[Joey Minta]

Can we see screenshots with category colors being used?

Comment 6

[Michiel van Leeuwen (email: mvl+moz@)]

Just some quick first thoughts on the screenshots:
- The border is not really visible on the top of the boxes. Would it work if we made the border a bit darker, so it's more visible?
- In the month view, there seems be a lot of unused space below the text. The bos is almost twice as high as it needs to be. Is that intentional?
- The end of the event in the week view is not clear. I think it is at the end of the shadow, but it looks to be at the end of the box, at the bottom border. Might be confusing. Not sure if it a big deal.
- I generally like the idea to make the boxes look better. nice work!

Comment 7

[Michael Büttner (no reviews TFN)]

Attachment: event with category

This is how an event with category is intended to look like. Please note that the proposed patch does not include this functionality. We have two options here 1) add the functionality to this patch 2) file a follow-up bug for this issue. I'd propose to take option 2 since the existing code for displaying the event boxes also doesn't take category colors into account.

Comment 8

[Michael Büttner (no reviews TFN)]

(In reply to comment #6)
> - The border is not really visible on the top of the boxes. Would it work if we made the border a bit darker, so it's more visible?
Christian and I tried loads of different attempts in order to make the events look nicer, the current look is what we came up with so far. Specifically the border colors are intentionally set as they are.

> - In the month view, there seems be a lot of unused space below the text. The
> bos is almost twice as high as it needs to be. Is that intentional?
Yes, it is intentional. But of course this is subject to discussion. Christian and I thought that it just looks more pleasant.

> - The end of the event in the week view is not clear. I think it is at the end
> of the shadow, but it looks to be at the end of the box, at the bottom border.
> Might be confusing. Not sure if it a big deal.
Unfortunately this is also something that I'm not totally happy with. You're right that this is a bit confusing. One solution would be to enlarge the boxes so that the shadow starts at the correct end-time. But in case there's another event starting at this particular end-time the boxes would be drawn as if they'd conflict. I don't have a clever solution to this problem, so I thought I propose that patch as it currently is in order to get the ball rolling.

> - I generally like the idea to make the boxes look better. nice work!
Thanks  ;-)

Comment 9

[Michiel van Leeuwen (email: mvl+moz@)]

I've played a bit with the patch. So i now have some more comments :) (In no particular order)

- The real thing looks better then the screenshot, due to jpg artifacts. Now the 'missing' border actually is not really a problem. I tried to lighten the inside of the box, but it didn't look any better.
- The labels are bold in both views. They used to not be bold. The bolding makes the text takes more space. We don't really have the room for that, especially in the monthview. I suggest removing the bold, for those practical reasons
- There should be a little space between the time and the label in the month view
- A rounded border might even look nicer, but I guess that's hard to do. So never mind that :)
- I still think that we should remove the large gap below the text in the month view. It takes too much space. Again, for practical reasons, I suggest removing it. We must balance between looking better and being more usable.
- I can no longer drag the top and bottom edges in the week view: 
Warning: reference to undefined property evbox.calendarView
Source File: chrome://calendar/content/calendar-multiday-view.xml
Line: 232
- The bottom-shadow problem can wait for now.

Comment 10

[Michael Büttner (no reviews TFN)]

Attachment: binary files v2

I modified the shadow images in order to make the events in the month view smaller. With the proposed patch in place, the height of an event box for all-day events and those that are displayed in the month view is determined by the height of the left/right shadow images, that's why i need to touch the images.

Comment 11

[Michael Büttner (no reviews TFN)]

Attachment: patch v2

This patch addresses the previous comments.

- bold labels > I removed the bold style, but spending the extra pixels for making the titles bold would be worth while. anyway, this shouldn't stop us from moving forward with this patch.
- little space between time and label in month view > I added a trailing *space* to the time string.
- I removed the large gap below the text in the month view. That's why I updated the binary files, since the height is determined by the height of the label and the height of the surrounding images, whatever is higher determines the final height of the box.
- Unfortunately there was a bug in the first version of the patch which caused the event boxes for all-day events to be as high as the gradient image. I fixed this problem with this version of the patch.
- dragging the grip-bars in the week view works again...

Comment 12

[bugzilla]

I think the patch for bug 273279 should be landed together with this bug, so we have all in one go.

Comment 13

[Stefan Sitter [:ssitter]]

Comment on attachment 254291 [details] [diff] [review]
patch v2

Looks nice. But please also add/package the new files to Sunbird.

Comment 14

[Michiel van Leeuwen (email: mvl+moz@)]

jminta pointed out that it would be nice if there was a border to indicate that you can drag the start and end time (only in day and week views, ofcourse)
I think that is why i asked about borders before. The current views have something simalar now.
Would it be possible to add some grippy like thing? Some relief that shows you can drag the borders.

Otherwise, i like the new views.

Comment 15

[Christian Jansen]

Attachment: Event with drag indicators

I support the idea of having grippies, but thes should only being displayed while mouse over, or if an event is selected (this reminds me that the default selection color should change). Anyhow, I've attached a mockup showing some drag indicators.

Comment 16

[Christian Jansen]

Attachment: revised look of grippies

Revised look of grippies. These should work much better. Grippies should be displayed while mouse is over an event.

Comment 17

[Michiel van Leeuwen (email: mvl+moz@)]

I like the look of the grippies. Do you want to implement them in this bug or in a followup? (assuming you want to implement them at all :) )

Comment 18

[Matthew (lilmatt) Willis]

> Created an attachment (id=254816) [details]
> revised look of grippies

+1!

Comment 19

[Michael Büttner (no reviews TFN)]

(In reply to comment #17)
> I like the look of the grippies. Do you want to implement them in this bug or in a followup?
I'm going to update this patch to include the grippies. ;-)

Comment 20

[Michael Büttner (no reviews TFN)]

Attachment: binary files v3

i hope this is the last revision of the binary files (at least concerning this patch) ;-) this time i included the grippy images.

Comment 21

[Michael Büttner (no reviews TFN)]

Attachment: patch v3

this version of the patch adds the grippies per previous comments. i also added the necessary stuff to make this work for sunbird. i hope that's it for now... ;-)

Comment 22

[Michiel van Leeuwen (email: mvl+moz@)]

I tried the patch in sunbird. I really like the way it looks now.
There is just one issue, still with the grippies. The border that you can use to dray is very small, just one or two pixel in height. That's quite hard to grab. And the grippie image is below that border, you can't grab that. That's a bit confusing.

Comment 23

[Michiel van Leeuwen (email: mvl+moz@)]

Comment on attachment 254291 [details] [diff] [review]
patch v2

I guess this patch v2 is obsolete now

Comment 24

[Michiel van Leeuwen (email: mvl+moz@)]

Attachment: partial patch

This patch show a possible solution to the grippy thing i mentioned. (partial patch, because i only made a diff of the two files i changed)
The main thing is that I moved the image of the grippy into the <calendar-event-gripbar/>. Then I changed the css to hide the image, instead of collapse it. That way, nothing moves when you mouse-over. The height stays the same.

Comment 25

[Markus Adrario [:Taraman]]

Very nice look. A real enhancement.

In the Month view all events have two lines now. I fear the view might get a little crowded if you have more than 3 events on one day.
How about having only one line for each entry?

Comment 26

[Michael Büttner (no reviews TFN)]

(In reply to comment #25)
> How about having only one line for each entry?
Thanks for the positive feedback ;-) You're completely right, the event boxes in the month view were too high. The latest version of the patch reduces the height to a single line.

Comment 27

[Michael Büttner (no reviews TFN)]

Attachment: patch v4

This is the revised version of this patch with better grippies. Instead of embedding the grippy image into the grip-bar (mvl, thanks for the partial patch), I decided to put the grip-bar including the images into the stack. This has the benefit that the size of the vertical border stays the same as the horizontal border, anything else would ruin the visual effect. mvl, i dropped the second review, as far as this is ok for you. i'm fine with the single-review model.

Comment 28

[Michiel van Leeuwen (email: mvl+moz@)]

Comment on attachment 255696 [details] [diff] [review]
patch v4

I tried that patch, and it all looks nice, and works nicely.
There is just the thing with the bottom of the box not being the actual end time (due to the shadows), but i think that can wait for another bug (but please do file one).

Comment 29

[Pete Riley]

I really like this new look of the event boxes in most views.  However, I think that they make the Month view look too cluttered, especially for days when I have a lot of events.

I hope that there will be a way for people to disable this new look in the month view at least.

Comment 30

[Michael Büttner (no reviews TFN)]

(In reply to comment #29)
> I hope that there will be a way for people to disable this new look in the
> month view at least.
Please note that the screen-shot for the month-view is no longer up to date (see comment #26 above). The items shown in the month view are now as larger as they were before (a single line of text) plus a 2 pixel shadow line below the boxes, so I'd assume that should be ok.

Comment 31

[Pete Riley]

Thanks for your reply.  The size of the event boxes in the month view is okay.

What I meant was that I'd like to be able to continue to override things like the background & category colors in UserChrome.css.

For example, in Sunbird at least, we can control these things on a per-category basis:

[item-category="Team Meeting"]
{ 
     color: black !important;
     background-color: lightgray !important;
     border-color: lightgray !important;
}

and it would be nice if this will continue to be as easy after the patch is implemented.

Also, it appears that the category color is now on the right side of the event box instead of a border around the box.  How would we reference that in UserChrome.css if it's no longer the 'border' element?

Comment 32

[Michiel van Leeuwen (email: mvl+moz@)]

Comment on attachment 255696 [details] [diff] [review]
patch v4

>+++ mozilla/calendar/base/content/calendar-month-view.xml	2007-02-19 17:19:30.468750000 +0100
>+			<xul:vbox flex="1">
You got tabs there. Make them spaces plaese. (Also in a lot of other places in this file)

>+            str = df.formatTime(val.startDate.getInTimezone(this.calendarView.mTimezone))+" ";

Instead of adding a (not realy flexible) space, you should change the css to add some room to the right. Then themes can changes that width if they so desire.

>+++ mozilla/calendar/base/content/calendar-multiday-view.xml	2007-02-19 17:07:06.796875000 +0100

>+        var evbox = this.parentNode.parentNode.parentNode.parentNode.parentNode.parentNode.parentNode;

That's quite ugly. But I don't know a better way either... :(

>+                <xul:image class="calendar-event-box-gradient"/>
>+								<xul:box xbl:inherits="orient" flex="1">
tabs

>+                  <xul:calendar-event-gripbar anonid="gripbar1" class="calendar-event-box-grippy-top"

The problem with having the gripbar here is that it causes problems when the label text doesn't fit in the box, because the box is not tall enough (because the event lasts only 15 minutes or so). Then the bottom gripbar can't be reached. But I also don't see an obvious solution here.


>+++ mozilla/calendar/base/content/calendar-view-core.xml	2007-02-19 15:38:59.296875000 +0100

>+#expand    skin/classic/calendar/gradient-overlay.png  (/calendar/resources/skin/classic/gradient-overlay.png)

You don't exand anything on this line, so no need for #expand. Or maybe, there is. But you need to use __THEME__ somewhere (do check syntax, i'm not sure of it)

Comment 33

[Matthew (lilmatt) Willis]

>>+++ mozilla/calendar/base/content/calendar-view-core.xml	2007-02-19 15:38:59.296875000 +0100
>>
>>+#expand    skin/classic/calendar/gradient-overlay.png  (/calendar/resources/skin/classic/gradient-overlay.png)
>
>You don't exand anything on this line, so no need for #expand. Or maybe, there
>is. But you need to use __THEME__ somewhere (do check syntax, i'm not sure of
>it)

Really, those items should be in /m/c/base/themes or we should (finally) create the /m/c/themes directory.  We don't want to add new stuff to /m/c/resources

Comment 34

[Michael Büttner (no reviews TFN)]

Admittedly, I don't know if you want me to prepare a new patch or if there's more to come. I don't want to prepare a new version of the patch just to get r- for something else.

Comment 35

[Simon Paquet [:sipaq]]

(In reply to comment #33)
> >>+#expand    skin/classic/calendar/gradient-overlay.png  (/calendar/resources/skin/classic/gradient-overlay.png)
> 
> Really, those items should be in /m/c/base/themes or we should (finally) create
> the /m/c/themes directory.  We don't want to add new stuff to /m/c/resources

Why don't we just create /m/c/base/themes right now and move the remaining stuff in /m/c/resources/classic over after we release 0.5.

Comment 36

[Michiel van Leeuwen (email: mvl+moz@)]

Comment on attachment 255696 [details] [diff] [review]
patch v4

Marking r- just for the record.
I don't have any other questions besides those in comment 32

Comment 37

[Michael Büttner (no reviews TFN)]

Attachment: binary files v4

I moved the binary files to /m/c/base/themes, as per previous comments and discussed on IRC.

Comment 38

[Michael Büttner (no reviews TFN)]

Attachment: patch v5

> The problem with having the gripbar here is that it causes problems when the
> label text doesn't fit in the box, because the box is not tall enough (because
> the event lasts only 15 minutes or so). Then the bottom gripbar can't be
> reached. But I also don't see an obvious solution here.
This one was a really though bastard and the reason why it took so long for this new revision of the patch. Sorry, that it took so long, but I was close to give up on this one. The solution is to calculate the maximum height the description tag can have and set this in setEditableLabel().

> Instead of adding a (not realy flexible) space, you should change the css to
> add some room to the right. Then themes can changes that width if they so
> desire.
I added the .calendar-month-day-box-item-label[time='true'], hope that's ok now. Please note that I also removed the spaces added to the string in the occurrence -setter in calemdar-month-view.xml (around line 117) since they had the same reason (adding space to the time-string). I just followed the prevailing strategy with adding the space to the time-string.

>+        var evbox = this.parentNode.parentNode.parentNode.parentNode.parentNode.parentNode.parentNode;
> That's quite ugly. But I don't know a better way either... :(
I left this as it was, if this is really something that bothers you we could step upwards the hierarchy until we hit an element with a specific anonid, localName, or something like this.

The rest of the comments have been addressed as requested.

Comment 39

[Michiel van Leeuwen (email: mvl+moz@)]

Comment on attachment 259096 [details] [diff] [review]
patch v5

The code looks good on my first look, except for the jar.mn changes. But i don't really know what they should be, so asking lilmatt for a second look.
Matt, can you also take a quick glance over the rest of the code, incase i missed anything obvious?

Comment 40

[Matthew (lilmatt) Willis]

Attachment: patch v6

My main problem with v4 was the jar.mn changes.  Since we're putting the PNG files in /m/c/base, we can use /m/c/base/jar.mn and it will be packaged with both Lightning and Sunbird.

I also made some style fixes, and changed CSS rules where you used "0px" to just "0", as "0" doesn't require a unit.

Comment 41

[Matthew (lilmatt) Willis]

Patch v6 and binary files v4 (in /m/c/base/themes/common) checked in on MOZILLA_1_8_BRANCH and trunk.

-> FIXED

Comment 42

[Omar Bajraszewski]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.4pre) Gecko/20070325 Calendar/0.5pre

Comment 43

[Omar Bajraszewski]

I wanted to say verified with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.4pre) Gecko/20070325 Calendar/0.5pre  (sorry)

Comment 44

[Patrick]

the event with category view does not seem to work. I added a new category with color, but no indication on the side of the event in the day, week or month view...Was this implemented, or taken out? 

The rest works by the way and looks perfect!! Good job all of you!
thanks.

Comment 45

[Michael Büttner (no reviews TFN)]

(In reply to comment #44)
> the event with category view does not seem to work.
This patch was not intended to include this functionality (see comment #7). There's already bug #340601 that will address this issue. Of course this won't make the 0.5 train, but it will be available shortly after that. Thanks for the positive feedback ;-)

Comment 46

[Tomaz]

Hey guys,
I think you are responsible for one annoying bug in few recent Sunbird nightly builds and I'm courious why nobody see it as very important
see
https://bugzilla.mozilla.org/show_bug.cgi?id=375390

Please take care of it before 0.5 release

Comment 47

[Stefan Sitter [:ssitter]]

(In reply to comment #46)
Tomaz, this bug is not present in Sunbird/Lightning 0.5pre nightly builds. 
Therefore it is not necessary to take care of it before 0.5 release.

Comment 48

[Vladimír Solnický]

It looks like this enhancement made my multiweek and month views unusable. As one field now uses half of the day box regardless of the amount of the text in the event. As I usually have 3-5 events a day, I cannot use these views to have an overall look any more. :'-(

Anyway, probably to late to cry here ...

My build is:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a4pre) Gecko/20070410 Calendar/0.6a1

Comment 49

[Michael Büttner (no reviews TFN)]

(In reply to comment #48)
Do you use a trunk build? In this case, please see bug #375390. It will be fixed after 0.5 has been released. If you're using a build from MOZILLA_1_8_BRANCH, please file a new bug and attach a screen shot.

Comment 50

[Simon Paquet [:sipaq]]

(In reply to comment #49)
> Do you use a trunk build? In this case, please see bug #375390. 

Michael, all Calendar/0.6a1 builds are trunk-based. So yes, Vladimír is seeing bug 375390. 

Comment 51

[Pete Riley]

It seems that the confusion about downloading a branch build versus a trunk build could be reduced if the branch builds were listed before the trunk builds on the download page at:
http://www.mozilla.org/projects/calendar/lightning/download.html

Comment 52

[Simon Paquet [:sipaq]]

Good suggestion, Pete.
Done!