413594 - move alarm indication icon to the extreme right

Status: VERIFIED FIXED

(Calendar :: Calendar Frontend, enhancement)

Description

[seshu.parvataneni]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.8) Gecko/20071009 SeaMonkey/1.1.5
Build Identifier: 2008011803

There is an alarm indication icon that gets pasted at the end of the event bar but before the space reserved for the category color. As a result the event name is truncated much earlier. If the alarm icon indicator is moved on top of the space that is reserved for the category color there would be a couple of more spaces left for displaying the name of the event. For users viewing calendars on a laptop or a small monitor (such as an Asus Eee PC) every bit of space is vital. So, I would request you to consider moving the alarm icon to the extreme right side of the event bar.

Reproducible: Always

Steps to Reproduce:
1. Create and event with a long name and an alarm to fire for that event
2. In the month view see where the alarm indication icon is displayed
3.
Actual Results:  
The alarm indication icon could be displayed on top of the category color. This could result in allocating more space to display the name of the event.

Expected Results:  
The alarm indication icon is pasted on top of the event bar truncating the event name much earlier

Comment 1

[seshu.parvataneni]

I am sorry in my description above the expected results are the actual results and the actual results should be the expected results. Thanks.

Comment 2

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

Confirming as per newsgroup reply from christian.

Comment 3

[seshu.parvataneni]

Attachment: some code changes with issue

I've modified the file a bit to move the alarm icon to the category box. There is an issue with this that I cannot figure out. The alarm icon in a month view is moved but the category box does not span to the extreme right side of the event box. There is some margin left. How do I fill that margin with the category box? I think similar change would go in appropriate view xul files.

This is my first stab at making changes to the calendar code. And I am pretty new to xul. I will also attach a screen capture of the problem that I am seeing. Any comments are greatly appreciated. Thanks.

Comment 4

[seshu.parvataneni]

Attachment: Image with the alarm icon moved to the category box

Please refer to my comments in #3. Thanks

Comment 5

[seshu.parvataneni]

can you please consider this for your 0.9 release? It would be nice to have feature since the usage has gone up for smaller laptop displays like the Eee pc. Thanks.

Comment 6

[Decathlon]

Attachment: Image with a normal and a smaller alarm icon

IMO it would be nice to have a smaller (less invasive) alarm icon too.
With a normal resolution monitor (1024x768) and with today pane in calendar-view, alarm icon takes much space and looks a bit oversized. A smaller size would help to free a bit of space for event text (one character) and would give a lighter look to the calendar-views.
In the screenshot there is a day box at its maximum width in week-view with today pane enable on a 1024x768 monitor resolution.

I posted here, without open a new bug, because it seems to me the goal is the same that is to free more space for name events in calendar view.

Comment 7

[seshu.parvataneni]

is this going to see the light any time soon? Please consider for your next release.

Comment 8

[Bas van den Bosch]

Do you have a real diff-file? 
https://wiki.mozilla.org/Calendar:Build

From attachment 299222 [details] isn't clear what lines were changed.

Also, you should ask for a reviewer:
https://wiki.mozilla.org/Calendar:Module_Ownership

Comment 9

[Jonatan Cloutier]

Attachment: move the bell over the category box

I've made some change to move the bell over the category box, the only thing is that the bell is a little larger than the box, so either the bell should be made smaller or the box larger.

I would tend to say that making the bell smaller would be best since we try to get a little bit more spacer here.

Comment 10

[Stefan Sitter [:ssitter]]

Attachment: screenshot with attachment 368760 applied

This patch breaks correct display of events without categories in addition to the issues mentioned in comment #9 for events with categories.

Comment 11

[Jonatan Cloutier]

Attachment: move the bell over the category box

I corrected the bug when there is no category, as well as bringing another issue with the category bar on a multiday view, but I cannot understand why. there do not seam to have any margin or padding that's doing it, if someone can look at it or I think the patch should be all good ( and after changing either of the picture)

Comment 12

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 368805 [details] [diff] [review]
 move the bell over the category box

I like what you've done, but when the alarm sits over the line like it does in the screenshot it looks like it has been misplaced.  

I would align it to the right as you've done, but increase the size of the category color area to match it.

Comment 13

[Jonatan Cloutier]

(In reply to comment #12)
> (From update of attachment 368805 [details] [diff] [review])
> I like what you've done, but when the alarm sits over the line like it does in
> the screenshot it looks like it has been misplaced.  
> 
> I would align it to the right as you've done, but increase the size of the
> category color area to match it.

as I've said in comment #9 " I would tend to say that making the bell smaller would be best since we try to get a little bit more spacer here."

but I can increase the size of the category box, for 2-3px it wont matter a lot.

I'll correct that as soon as I find time to work on the issue in comment #11

Comment 14

[Decathlon]

Attachment: problem when box events shrink

There is another problem with the patch when events shrink (e.g. when there are many events with overlapped time in the same day in week-view), the bell and the category box disappear (see attachment).

Comment 15

[Decathlon]

Attachment: "sub-patch" for problem in comment #11 and #14. No review required.

Looking into problem in comment #11 I've modified calendar-multiday-view.xml and the attached patch seems to solve problem in comment #14 too. Not sure if there are others side effects.
Could you try?
I posted it as a patch only to avoid wrap line on bugzilla.

Comment 16

[Jonatan Cloutier]

(In reply to comment #15)
> Created an attachment (id=371135) [details]
> "sub-patch" for problem in comment #11 and #14. No review required.
> 
> Looking into problem in comment #11 I've modified calendar-multiday-view.xml
> and the attached patch seems to solve problem in comment #14 too. Not sure if
> there are others side effects.
> Could you try?
> I posted it as a patch only to avoid wrap line on bugzilla.

did you test with the first or the second patch ? cause you seam to have done change on the first one, and the patch you give look exactly as my second patch and I can't apply it, tough I might I've miss something...

Comment 17

[Decathlon]

(In reply to comment #16)
> did you test with the first or the second patch ? cause you seam to have done
> change on the first one, and the patch you give look exactly as my second patch
> and I can't apply it, tough I might I've miss something...

I used the second patch (attachment 368805 [details] [diff] [review]) but I changed calendar-multiday-view.xml code (only that part) with that one in my patch.
New patch should have code of your patch for calendar-month-view.xml and calendar-view-core.xml files, and code of attachment 371135 [details] [diff] [review] for calendar-multiday-view.xml file (I called it "sub-patch" for this reason ;-).

It seems to me that code about 'stack' element should be pasted in the lines from 1913 to 1920 instead of lines 1912-1919 and line 1905 shouldn't change.
I've just tried on a new profile and it seems working on week view although I haven't fully tested it.
Sorry for my bad explanation in comment #15.

Comment 18

[Jonatan Cloutier]

thank for your explanation, it's look like it's working here too, I'll test a bit more and change the category bar size and I'll post the patch if everything is good.

Comment 19

[Decathlon]

About category-box size, could you reconsider the idea of enlarge it? I know I've already said that (see comment #6) but the goal of this bug is to free space for text in the event boxes. Moving the alarm icon to the right we get it, but enlarging category bar we will also reduce space when there is only category box without alarm icon.

For sure even few pixels will reduce some character in event text. I always think to month-view with todaypane open: in the event-boxes there are event time and category box, so the space for event title is very little and the 'crop' attribute replaces some character with ellipsis too. In week-view the problem is different because text is covered by category box (instead, Lightning 0.9, shows the text even over the category box), so, a larger category stripe, will cover more text and this is more important when there are events in the same day with overlapped time because boxes become half large.

If isn't possible to have a smaller alarm icon, for a better look and avoid the sense of misplacement as reported in comment #12, could you consider to reduce, instead of enlarge, the category box? Maybe a box with his left border exactly in the middle of the alarm icon could be a good compromise. Just an idea.

Comment 20

[Stefan Sitter [:ssitter]]

Please don't forget to retest with advanced preference "calendar.alarms.indicator.show" set to "false" too.

Comment 21

[Jonatan Cloutier]

(In reply to comment #19)
> About category-box size, could you reconsider the idea of enlarge it? I know
> I've already said that (see comment #6) but the goal of this bug is to free
> space for text in the event boxes. Moving the alarm icon to the right we get
> it, but enlarging category bar we will also reduce space when there is only
> category box without alarm icon.

I totally agree with you, the only thing is that I cannot change the bell picture to resize it, the best I could do is resizing it by it's property and that's not gonna look good. 

> If isn't possible to have a smaller alarm icon, for a better look and avoid the
> sense of misplacement as reported in comment #12, could you consider to reduce,
> instead of enlarge, the category box? Maybe a box with his left border exactly
> in the middle of the alarm icon could be a good compromise. Just an idea.

I'm not sure of what the result would look like, I might try it and do a screen shot for it to be reviewed before.

Comment 22

[Jonatan Cloutier]

Attachment: Category bar reducted

here is what it's look like when we reduce the Category bar to the middle of the bell, 
-do we keep it like that
-do we resize it to be larger than the bell
-do we resize the bell for it to be smaller

Comment 23

[Jonatan Cloutier]

(In reply to comment #20)
> Please don't forget to retest with advanced preference
> "calendar.alarms.indicator.show" set to "false" too.

bug 487400 reported that the property isn't working at the moment, so I'm waiting for it to be reviewed/committed to answer your question.

Comment 24

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 371478 [details]
Category bar reducted 

> here is what it's look like when we reduce the Category bar to the middle of
> the bell, 
> -do we keep it like that
> -do we resize it to be larger than the bell
> -do we resize the bell for it to be smaller

I like the 1/2 bell size in terms of space saving and by picking a defined area (1/2 bell size) it looks much more placed than before when it looked like it was floating in error.

Comment 25

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

Comment on attachment 368805 [details] [diff] [review]
 move the bell over the category box

Codewise this looks fine, but I'm a bit confused what the final patch should be? The patch doesn't look like the category bar is changed to half the alarm icon size? Does the category bar in the screenshot still have the gradient the old bar had?

r- for now to get an updated patch that is ready for review.

Comment 26

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

Please consider also fixing bug 453067 with this patch if possible.

Comment 27

[Jonatan Cloutier]

Attachment: move the bell over the category box - final

here is the patch with all correction, please test if the all day box render well in multiweek and day view because I had strange behaviour with it.

I cannot patch the bug 45307 since I'm not able to reproduce the issue

Comment 28

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 374494 [details] [diff] [review]
 move the bell over the category box - final

haven't played with this  so I'm assuming it's working as described.

Comment 29

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

Comment on attachment 374494 [details] [diff] [review]
 move the bell over the category box - final

Works fine, I experienced no problems in any view, neither in sunbird nor lightning.

Sorry for the late review!

Comment 30

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

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

-> FIXED

Comment 31

[seshu.parvataneni]

I wish the category tab that is displayed just below the alarm icon is wider. Since now the alarm icon is pasted over the category tab the category tab is barely visible.

Comment 32

[Peter Lairo]

Attachment: Screenshot: Alarm icon now too low!?

It seems the alarm icon is now too low. It looks like the icon is only partially displayed.

Perhaps a few more pixels space along the bottom to better "frame" the icon.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1pre) Gecko/20090618 Lightning/1.0pre Shredder/3.0b3pre

Comment 33

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

Yes, that does seem to low. Interesting though, I couldn't reproduce this. Maybe you can attach that set of events as a calendar and describe the way your window is layed out?

Comment 34

[Decathlon]

On 2009-06-16 (after the patch was checked in) it worked well with WinXP.
There is another problem about 'crop' behavior of event title: when view shrinks, title disappear word by word instead of character by character.

Regression range:
Lightning 1.0 20090616034158 works
Lightning 1.0 20090617045110 fails
check-in during regression range:
https://hg.mozilla.org/comm-central/pushloghtml?startdate=2009-06-16+03%3A41%3A58+&enddate=2009-06-17++04%3A51%3A10

maybe one of these?
https://hg.mozilla.org/comm-central/rev/d4e0ef5c3687
https://hg.mozilla.org/comm-central/rev/3ed4b7db765e
https://hg.mozilla.org/comm-central/rev/ea1db982e51e

Comment 35

[seshu.parvataneni]

Attachment: category tab not visible with alarm icon displaced

Comment 36

[seshu.parvataneni]

To reproduce the cropped alarm icon you need to have a long event name that does not fit in the display. From the attached image you can see that the category tab is not clearly visible.

Comment 37

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

If possible we should fix this for the beta, but I'd be ok with moving this to beta2 if we don't have enough time.

Comment 38

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

Caused by bug 273279, backing this one out locally fixes the issue for me.

Comment 39

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

I have a local patch that fixes the issue and also greatly simplifies the xul needed for our month boxes. This gave me quite a performance win. If I finish this in time, I'll attach it to a new bug and then close this one again.

Comment 40

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

This bug is FIXED, the actual regression is caused by bug 273279. The regression will be fixed with bug 501302.

Comment 41

[Damian Szczepanik]

verified with
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.9pre) Gecko/20100225 Calendar/1.0b2pre

Comment 42

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

These bugs are likely targeted at Lightning 1.0b1, not Lightning 1.0. If this change was done in error, please adjust the target milestone to its correct value. To filter on this bugspam, you can use "lightning-10-target-move".