306693 - Suggestion: Put "Go to Today" icon on the Calendar toolbar

Status: VERIFIED FIXED

(Calendar :: Sunbird Only, defect)

Description

[石庭豐 (Seak, Teng-Fong)]

It would be nice if the "Go to Today" icon would be displayed as a default icon
together with "Day view", "Week view", etc on the Calendar toolbar.  At least,
personally I use it very often.  Everytime I do some test which needs erasing
everything, I have to re-assigned it to the toolbar.  In the end, it's very tedious.

Comment 1

[Joey Minta]

The calendar extension has a 'Go to today' button below the minimonth datepicker
on the left side of the window.  That's another possible option to consider
here.  Whatever we do, it ought to work towards bringing the two xul's into more
agreement.

Comment 2

[Simon Paquet [:sipaq]]

(In reply to comment #1)
> The calendar extension has a 'Go to today' button below the minimonth datepicker
> on the left side of the window.  That's another possible option to consider
> here.  Whatever we do, it ought to work towards bringing the two xul's into more
> agreement.

For Sunbird the "Go to Today" button in the minimonth datepicker was
deliberately removed in bug 256405. The participating parties (mvl, mostafah and
me) were actually a bit unsure, whether it should be on by default as you can
see from the comment section of bug 256405.

Comment 3

[Simon Paquet [:sipaq]]

-> me
Patch coming on the weekend

Comment 4

[Simon Paquet [:sipaq]]

Attachment: Patch v1

Here's the fix for this bug. Since this is a UI decision I would like to have a
second review by mvl before this could be checked in.

Justification for the patch:
This functionality existed until it was removed in bug 256405. But we
erroneously thought (in that bug) that this is not a basic UI feature of
comparable apps like Outlook, which do have this feature available in their
default UI configuration.

Comment 5

[Joey Minta]

Comment on attachment 194795 [details] [diff] [review]
Patch v1

The code itself looks fine.  I think I might prefer the button at the end of
the toolbar set, though.  To me, it's just more intuitive there.  Where does
Outlook put it?  Either way, I'll wait for mvl's opinion on this, since I don't
really have a good reason.

Also, line-wrapping at 80 chars is good.  Oddly, to do it here you have to do:

defaultset="foo1,foo2,foo3,
,foo4,foo5,foo6,
,foo7,foo8"

Which style is uglier? Again, maybe mvl has an opinion.

Comment 6

[石庭豐 (Seak, Teng-Fong)]

"Today" is before other view in Outlook.

I've just had a look at the patch.  Jeez, the icons are defined inside a string!
 Not easy to read or to make correction.  I'd expect something like:

<toolbar class="toolbar-primary chromeclass-toolbar" id="calendar-bar" ......> 
           
 <icon item="calendar-new-event-button"/>
 <icon item="calendar-new-task-button"/>
 .....
 <icon item="separator"/>
 .....
 .....

</toolbar>

If you're going to change xul, please think about this for maintainability's sake.

Comment 7

[Simon Paquet [:sipaq]]

Teng-Fong, the icons are defined in the way that you are proposing. See
http://lxr.mozilla.org/mozilla/source/calendar/sunbird/base/content/calendar.xul#284


The line in the patch just contains the instructions for the buttons, which are
on by default.

Joey, I know the style is ugly and not very maintainable. I just left it as it
is in this patch and also because I think the wrapped variant is not really an
improvement here. But of course I will adjust the patch, if you or mvl think
it's for the better.

Comment 8

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

Comment on attachment 194795 [details] [diff] [review]
Patch v1

Maybe you can make it more readable by putting every item on it's own line?
And i think the location is fine, but i don't have a strong opinion. If jminta
likes it to go at the end, let's go for that.
r=mvl anyway, cleanups are optional :)

Comment 9

[Simon Paquet [:sipaq]]

Joey, can I get your review please, now that mvl gave his.

I think the style of that particular line should be changed according to mvl's
suggestion in comment 8.

I'm fine with putting the button wherever you want it to, but as Teng-Fong
rightly pointed out, Outlook places it before the views and encapsulates it
between separators just like it is done in the patch.

Please let me know, so that I can update my patch.

Comment 10

[Joey Minta]

Comment on attachment 194795 [details] [diff] [review]
Patch v1

r=jminta if that's where outlook puts it.

Every other appearance of defaultset in mozilla's codebase seems to keep the
list on one line, so let's just leave it.

Comment 11

[Joey Minta]

patch checked in

Comment 12

[Simon Paquet [:sipaq]]

verified fixed