370836 - Making mainWindow title for day view more detailed

Status: VERIFIED FIXED

(Calendar :: Calendar Frontend, enhancement)

Description

[Omar Bajraszewski]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; pl-PL; rv:1.8.1.1) Gecko/20061204 Firefox/2.0.0.1
Build Identifier: 

MainWindow title for day view should show exact date i.e. "Saturday, 17 Feb"

Reproducible: Always

Comment 1

[Omar Bajraszewski]

Markus agreed to take care of it. Lilmatt suggested me open a new bug for it :-)

Comment 2

[Markus Adrario [:Taraman]]

I will do so... :-)

Comment 3

[Markus Adrario [:Taraman]]

Attachment: Adding the Date to title in Day-View

For today it would look like this with my date settings:
"Wednesday, 23 Feb - Calendar"

I had to add a new parameter to the "setNames"-Method, so I get the actual date.
For now I only changed the function call in "calendar-decorated-day-view.xml" to add the new parameter. Might there be a problem with an uninitialized value when calling from the other decorated views with only one parameter?

I omitted the year for now, to keep the line short as I think one should know which year he is in.
For now the weekday name is taken from the name Array which makes it dependent on the Calendar locale and not the Computers Date/Time Settings.

Comment 4

[Joey Minta]

Comment on attachment 255951 [details] [diff] [review]
Adding the Date to title in Day-View

+                    if(!aDate) {
Style nit: there should be a space after logical operators (if, while, for)

+                      document.title = aNameArray[2] + " - " + brand.GetStringFromName("brandShortName");  
Style nit: 4 space indenting please.  Also, there are some trailing spaces on this line.

+                    } else {
+                      var df = Components.classes["@mozilla.org/calendar/datetime-formatter;1"]
+                                         .getService(Components.interfaces.calIDateTimeFormatter);
We have this cool trick where Components.classes is just Cc and Components.interfaces is Ci.  That will shorten these lines and make them more readable.

+                      document.title = aNameArray[2] + ", " + df.formatDateWithoutYear(aDate) + " - " + 
Why without year?  Anyway, that's a question for the ui-folks.

I'm curious, however, whether we ought to just put the selected day in the title bar.  That would accomplish the same thing as this patch, for the day view, and would add consistency for the other views.  Just a thought...

Comment 5

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

Comment on attachment 255951 [details] [diff] [review]
Adding the Date to title in Day-View

You can leave in the year. Look at firefox, the title is pretty long there. Those 5 extra characters won't hurt.
ui-review=mvl with that

Comment 6

[Markus Adrario [:Taraman]]

Attachment: Fixed the things, joey mentioned

>>+                    if(!aDate) {
>>Style nit: there should be a space after logical operators (if, while, for)
thats easy... ;-)

>>+                      document.title = aNameArray[2] + " - " +
>>brand.GetStringFromName("brandShortName");  
>>Style nit: 4 space indenting please.  Also, there are some trailing spaces on
>>this line.
whoa, I thought I found all these... --> done.

>>+                    } else {
>>+                      var df =
>>Components.classes["@mozilla.org/calendar/datetime-formatter;1"]
>>+                                        
>>.getService(Components.interfaces.calIDateTimeFormatter);
>>We have this cool trick where Components.classes is just Cc and
>>Components.interfaces is Ci.  That will shorten these lines and make them more
>>readable.
I must admit: I copied most parts of this code from another file, so I didn't know. But its a great thing.

+                      document.title = aNameArray[2] + ", " +
df.formatDateWithoutYear(aDate) + " - " + 
Why without year?  Anyway, that's a question for the ui-folks.
Since Michiel stated the same thing, I tried it and it looked good.

Comment 7

[Matthew (lilmatt) Willis]

Comment on attachment 257250 [details] [diff] [review]
Fixed the things, joey mentioned

>Index: calendar-decorated-base.xml
>===================================================================
>-                    var sbs = Components.classes["@mozilla.org/intl/stringbundle;1"]
>-                                        .getService(Components.interfaces.nsIStringBundleService);
>+                    var sbs = Cc["@mozilla.org/intl/stringbundle;1"].getService(Ci.nsIStringBundleService);
Don't bother changing this since you don't need to touch it.  Doing so just makes cvs blame harder to read.

>                     var brand = sbs.createBundle("chrome://branding/locale/brand.properties");
> 
>-                    document.title = aNameArray[2] + " - " + brand.GetStringFromName("brandShortName");
>+                    if (!aDate) {
>+                        document.title = aNameArray[2] + " - " + brand.GetStringFromName("brandShortName");
>+                    } else {
>+                        var df = Cc["@mozilla.org/calendar/datetime-formatter;1"]
>+                                    .getService(Ci.calIDateTimeFormatter);
>+                        document.title = df.formatDate(aDate) + " - " + brand.GetStringFromName("brandShortName");
>+                    }
I'd like to remove the code duplication.

How about:

    var docTitle;
    if (!aDate) {
        docTitle = aNameArray[2];
    } else {
        var df = Cc["@mozilla.org/calendar/datetime-formatter;1"].
                 getService(Ci.calIDateTimeFormatter);
        docTitle = df.formatDate(aDate);
    }
    
    document.title = docTitle + " - " + brand.GetStringFromName("brandShortName");

Note the Cc and . alignment.



r=lilmatt with that fix.

Comment 8

[Markus Adrario [:Taraman]]

Attachment: Windowtitle with Date in Day-View + Year

OK, you're right matt. changed that
I decided to keep the short Cc and Ci since the long forms were from my own last patch to that file.
Hope that is ok.

Comment 9

[Matthew (lilmatt) Willis]

Comment on attachment 257766 [details] [diff] [review]
Windowtitle with Date in Day-View + Year

Removing r2 request.  No longer needed.

Comment 10

[Matthew (lilmatt) Willis]

The long forms were in the file at this point, so we should keep them unless there's a good reason to change them.

Thanks for the patch.

Patch checked in (without the Components.classes to Cc change on stringbundleService) to MOZILLA_1_8_BRANCH and trunk.

-> FIXED

Comment 11

[Markus Adrario [:Taraman]]

Oooops,

on creating the last patch I forgot something really important:

in calendar-decorated-base.xml: <parameter name="aDate"/>

Matt, please add that...

Thx

Comment 12

[Matthew (lilmatt) Willis]

This was already fixed.

-> FIXED

Comment 13

[Damian Szczepanik]

verified with
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a3pre) Gecko/20070308 Calendar/0.6a1

Comment 14

[Damian Szczepanik]

Litmus testcase 3009 created