369848 - Full name of day in Dayview (or none)

Status: VERIFIED FIXED

(Calendar :: Calendar Frontend, enhancement)

Description

[Foppe Benedictus]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; nl; rv:1.8.1) Gecko/20061010 Firefox/2.0
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; nl; rv:1.9a1) Gecko/20070208 Sunbird/0.3.1

The dayview shows under the date the shortname of the day (Mon, Tue, Wed etc). There is space enough to display the full name. But because the full name is already in dayview navigation, I think the dayname could be left out at all.

Reproducible: Always

Steps to Reproduce:
1. In Dayview window

Comment 1

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

Attachment: long weekday names - v5

Wow this bug was a real b*tch, I almost gave up on it. Anyway, this patch takes care of it. I hope it also works on mac.

Comment 2

[Michael Büttner (no reviews TFN)]

Comment on attachment 268413 [details] [diff] [review]
long weekday names - v5

>+      <constructor><![CDATA[
>+      ]]></constructor>
Why keep an empty constructor?

>+          long.setAttribute("value", calGetString("dateFormat","day." + (val.weekday + 1) + ".name"));
>+          short.setAttribute("value", calGetString("dateFormat","day." + (val.weekday + 1) + ".Mmm"));
You could also write "long.value = calGetString..." and "short.value = calGetString...". I personally find this a bit more concise.

>+            this.longWeekdayPixels = long.boxObject.width +
>+                                     parseInt(document.defaultView.getComputedStyle(long, null).getPropertyValue("margin-left")) +
>+                                     parseInt(document.defaultView.getComputedStyle(long, null).getPropertyValue("margin-right"));
That's cool, thanks for teaching me this neat little trick :-)

>               if (counter < labelboxkids.length) {
>                   labelbox = labelboxkids[counter];
>-                  labelbox.firstChild.setAttribute("value", df.formatDateWithoutYear(d));
>-                  labelbox.childNodes[1].setAttribute("value", calGetString("dateFormat", "day."+ (d.weekday+1)+ ".Mmm"));
>+                  labelbox.date = d;
>+                  labelbox.shortWeekNames = false;
>               } else {
>-                  labelbox = createXULElement("vbox");
>+                  labelbox = createXULElement("calendar-day-label");
>                   labelbox.setAttribute("flex", "1");
>-                  labelbox.setAttribute("class", "calendar-day-label-box");
>-
>-                  var label = createXULElement("label");
>-                  label.setAttribute("value", df.formatDateWithoutYear(d));
>-                  label.setAttribute("class", "calendar-day-label-date");
>-                  labelbox.appendChild(label);
>                   labeldaybox.appendChild(labelbox);
> 
>-                  label = createXULElement("label");
>-                  label.setAttribute("value", calGetString("dateFormat", "day."+ (d.weekday+1)+ ".Mmm"));
>-                  label.setAttribute("class", "calendar-day-label-name");
>-                  labelbox.appendChild(label);
>-              
>-                  labeldaybox.appendChild(labelbox);
>+                  labelbox.date = d;
>+                  labelbox.shortWeekNames = false;
>               }
I'd write it like so (pseudo-code):
  if (labelboxkids.length < counter)
    create new element
  initialize the element
But that's of course personal preference.

Great work, clean and elegant solution. Thanks for the patch, r=mickey. If you're so inclined you could address the nits I mentioned above and post a new iteration of the patch. But since these were just minor bits and pieces I'm equally confident with going ahead as it currently stands.

Comment 3

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

(In reply to comment #2)
> Why keep an empty constructor?
I'll take it out when committing the patch. I probably had some code in there
at some point.

> You could also write "long.value = calGetString..." and "short.value =
> calGetString...". I personally find this a bit more concise.
I'd like to use setAttribute to keep with the style of the file. value is
always set using setAttribute there.

> I'd write it like so (pseudo-code):
>   if (labelboxkids.length < counter)
>     create new element
>   initialize the element
> But that's of course personal preference.
I'd like to keep it the way it is for the same reason. There are many places
where its done like that so I think it should stay :)

Comment 4

[Christian Jansen]

Comment on attachment 268413 [details] [diff] [review]
long weekday names - v5

r=Christian
It looks good for me. In long term we should optimize the look by adjusting colors and font size, but this is a different task.

Comment 5

[Stefan Sitter [:ssitter]]

Seems this patch was checked in yesterday and caused Bug 386336.

Comment 6

[Omar Bajraszewski]

Verified with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.5pre) Gecko/20070702 Calendar/0.7pre and Lightning 2007070203