Closed
Bug 444292
Opened 16 years ago
Closed 16 years ago
Replace Decorated Header (Nav Bar) by standard Items
Categories
(Calendar :: Calendar Frontend, defect)
Calendar
Calendar Frontend
Tracking
(Not tracked)
VERIFIED
FIXED
0.9
People
(Reporter: berend.cornelius09, Assigned: berend.cornelius09)
References
Details
(Keywords: late-l10n)
Attachments
(9 files, 1 obsolete file)
997 bytes,
image/png
|
Details | |
102.73 KB,
image/png
|
chris.j.bugzilla
:
ui-review+
|
Details |
1022 bytes,
image/png
|
Details | |
87.62 KB,
patch
|
Details | Diff | Splinter Review | |
4.17 KB,
patch
|
Fallen
:
review+
|
Details | Diff | Splinter Review |
2.12 KB,
patch
|
Fallen
:
review+
|
Details | Diff | Splinter Review |
1.03 KB,
patch
|
Fallen
:
review+
|
Details | Diff | Splinter Review |
2.50 KB,
patch
|
dbo
:
review+
|
Details | Diff | Splinter Review |
22.02 KB,
patch
|
Fallen
:
review+
chris.j.bugzilla
:
ui-review+
|
Details | Diff | Splinter Review |
As being discussed on the face2face meeting in Hamburg (see http://wiki.mozilla.org/Calendar:Hamburg_2008_-_F2F_Meeting) we want to redesign our views (see http://wiki.mozilla.org/Calendar:Improving_the_Calendar_Views and http://wiki.mozilla.org/Calendar:Lightning09#Calendar_Views The new decorated header is to contain navigation buttons, date Info and Calendar Week Info and should appear more "tidy up than the current decorated header.
Flags: wanted-calendar0.9?
Assignee | ||
Comment 1•16 years ago
|
||
This png contains the images for the navigation buttons and conforms to the navigation buttons already used in the miniday and minimonth panes. It must be copied into the calendar/base/themes/common/widgets folder
Assignee | ||
Comment 2•16 years ago
|
||
first version of this issue. I already gave it to Andreas to test it and he found it Ok. The header is implemented for the rotated view, too. What is still improveable is the display of the date ranges on the left hand side of the decorated header. I implemented it with our "ordinary" "formatInterval" function of the "datetime-formatter" but Christian (and me too) was not satisfied with the result. I'd like to find a better solution in a second patch.
Attachment #328640 -
Flags: ui-review?(christian.jansen)
Attachment #328640 -
Flags: review?(philipp)
Assignee | ||
Comment 3•16 years ago
|
||
screenshot of the new decorated header with my first patch being applied.
Assignee | ||
Comment 4•16 years ago
|
||
Christian gave me some instructions on how to format the date-range display: ... ~~~ Day ~~~ EN: March 3, 2008 DE: 3. März 2008 ~~~ Week ~~~ EN: March 3 - 9, 2008 DE: 3 - 9. März 2008 EN: September 29 - October 5, 2008 DE: 29. September - 5.- October 2008 EN: December 29, 2008 - January 4, 2009 DE: 29. Dezember 2008 - 4. Januar 2009 ~~~ Multiweek (in this example 2 weeks) ~~~ EN: March 3 - 16, 2008 DE: 3 - 16. März 2008 EN: September 29 - October 12, 2008 DE: 29. September - 12.- Oktober 2008 EN: December 29, 2008 - January 11, 2009 DE: 29. Dezember 2008 - 11. Januar 2009 ~~~ Month ~~~ EN: March 2008 DE: März 2008 For the hyphen we should use "–" or in Unicode "–" ... I am not sure what kind of localization problems come up when we implement these formats and would therefor welcome input from the community. I am going to get some advice from an OpenOffice-Calc developer in the coming days. Probably I am also going to modify "formatInterval" in the file calendar/base/src/calDateTimeFormatter.js which would mean that I have to adapt the client code at several other places.
Updated•16 years ago
|
Flags: wanted-calendar0.9? → wanted-calendar0.9+
Comment 5•16 years ago
|
||
(In reply to comment #4) If you are modifying "formatInterval" please consider Bug 413103 too. Regarding localization problems: The interval string should probably defined in a l10n file, e.g. like "%1 - %2". This way the l10n teams could swap the order of start and end date or replace the "-" with a more appropriate character.
Comment 6•16 years ago
|
||
I think the first calendar weeks in the year should be displayed in two digits format. Because my first impression when I traveled in the multiweekview across the year change and I saw 'Calendar weeks 51-2' was "Oh, the leading 5 is gone -> bug". To prevent this it would be better to display a '02'.
Comment 7•16 years ago
|
||
Not that I do ui-review, but I dislike buttons that are gray in their normal state. While its fine to have hover/active be a different color, I think its visually distracting to have it always be gray.
Status: NEW → ASSIGNED
Comment 8•16 years ago
|
||
Comment on attachment 328640 [details] [diff] [review] patch v. #1 > skin/classic/calendar/widgets/nav-buttons.png (themes/common/widgets/nav-buttons.png) >+ skin/classic/calendar/widgets/view-navigation.png (themes/common/widgets/view-navigation.png) These images are obviously quite similar. I'd prefer to have them in one file when checking in, especially since the filenames are quite similar too. >+ tooltiptext="previous day" >+ tooltiptext="today" >+ tooltiptext="next day" Please get rid of hardcoded strings. I've noticed some tooltip text gets set later on, so we probably don't need those strings anyway? >+ secondWeekNo = getWeekFormatter().getWeekTitle(aEndDate); This line has redundant spaces, there was at least one other line somewhere else. >+ document.getAnonymousElementByAttribute(this, "anonid", "nav-control").setDateRange(aStartDate, aEndDate, aToolTipTexts, viewElement); Wrapping would be nice here. >+ d1.day += 7*aNumber; Spaces around operator >+ this.fireEvent("relayouted"); I think just "relayout" sounds more natural, but thats up to you. >+function calGetStringArray(aBundleName, aStringNames, aParams, aComponent) { Documentation please. > calendar-navigation-buttons { >+ padding-top: 7px; >+ padding-bottom: 4px; >+ background-color: white; white? isn't there some -moz-... color we can use here? >+.view-header { >+ font-weight: bold; >+ font-size: 15px; >+ color: #616163; Same question here? looks almost like gray, so maybe also a -moz color? >+.view-navigation-button { >+ list-style-image: url("chrome://calendar/skin/widgets/view-navigation.png"); >+ -moz-user-focus: normal; >+} Quotes around the url are not needed >Index: locales/en-US/chrome/calendar/global.dtd ... >+<!ENTITY onemonthbackward.tooltip "One Month Back" > >+<!ENTITY onemonthforward.tooltip "One Month Forward" > >+<!ENTITY showToday.tooltip "Go to Today"> >+<!ENTITY onedayforward.tooltip "One Day Forward"> >+<!ENTITY onedaybackward.tooltip "One Day Backward"> >+<!ENTITY showselectedday.tooltip "Show events for selected day"> Do we really use these globally? Or are they maybe better kept in some view specific .dtd ? r=philipp
Attachment #328640 -
Flags: review?(philipp) → review+
Comment 9•16 years ago
|
||
> >+ tooltiptext="previous day"
> >+ tooltiptext="today"
> >+ tooltiptext="next day"
> Please get rid of hardcoded strings. I've noticed some tooltip text gets set
> later on, so we probably don't need those strings anyway?
>
I suggest to use more instructing tooltips like:
Mini Day
One Day Back
Go to Today
One Day Forward
Mini Month
One Month Back
Go to Today
One Month Forward
Day View
One Day Back
Go to Today
One Day Forward
Week View
One Week Back
Go to Today
One Week Forward
Multiweek
One Week Back
Go to Today
One Week Forward
Month
One Month Back
Go to Today
One Month Forward
Comment 10•16 years ago
|
||
(In reply to comment #7) > Not that I do ui-review, but I dislike buttons that are gray in their normal > state. While its fine to have hover/active be a different color, I think its > visually distracting to have it always be gray. > Good point. I think I'll replace them by black nav buttons.
Assignee | ||
Comment 11•16 years ago
|
||
in reply to commment #9: The hardcoded strings are merely relicts from the first implementation phase and I just forgot to remove them. My patch already contains the strings from Christian.
Comment 12•16 years ago
|
||
(In reply to comment #10) > (In reply to comment #7) > > Not that I do ui-review, but I dislike buttons that are gray in their normal > > state. While its fine to have hover/active be a different color, I think its > > visually distracting to have it always be gray. > > > > Good point. I think I'll replace them by black nav buttons. See attachment >
Comment 13•16 years ago
|
||
Comment 14•16 years ago
|
||
Comment on attachment 328641 [details]
screenshot
Looks good for me.
Please try to fix the problem that the buttons jump if one switches from "Week" to "Day", or "Today". Please also change the color used for the date info to #2E4E73.
The date formating should be fixed in a spin-off bug.
ui=christian
Attachment #328641 -
Flags: ui-review+
Assignee | ||
Comment 15•16 years ago
|
||
I worked in the comments as suggested by Philipp and Christian. Some notes: >These images are obviously quite similar. I'd prefer to have them in one file >when checking in, especially since the filenames are quite similar too. I don't think this is a good idea since the images have different sizes, so that the moz-image-region property of all images has to be adapted each time one of the images has been modified. >white? isn't there some -moz-... color we can use here? As far as I understood http://developer.mozilla.org/en/docs/CSS:color_value these common color keyworks are the ones to be used. > Same question here? looks almost like gray, so maybe also a -moz color? No Christian insists to use his own color definitions. In this case the font-color has to be adapted to the image color. >Do we really use these globally? Or are they maybe better kept in some view >specific .dtd ? I was unsure myself. I used the global.dtd because it was the only one that was already used by the minimonth. On the other hand I find these tooltips "globally" usable for the future. I tried to address the "jumping" of comment #18 by setting the padding *before* the label was set and found this improved the situation partly. What else can I do? One last note about trailing whitespaces. I have recently tended to remove them shortly before the check-in and then remove all whitespaces from the files with the respective function of my editor - at least with files that are not modified frequently. Removing them before the review makes the patch unnecessary big, unclear and likely to get bitrotten. I hope this is Ok and of course I still appreciatereviewers pointing to that problem. I removed the "prevnextarrow.png" files in the base/themes/pinstripe|winstripe folders as they are no longer needed modified patch checked in on trunk and MOZILLA_1_8_BRANCH ->issue remains open
Assignee | ||
Comment 16•16 years ago
|
||
patch that I just checked in. I forgot to mention that I added tooltips to the navigation buttons in the minimonth and adapted the tooltips in the miniday to Christian's string recommendation.
Updated•16 years ago
|
Attachment #328640 -
Flags: ui-review?(christian.jansen) → ui-review+
Comment 17•16 years ago
|
||
@Berend: I checked this patch today in lightning 2008071020 and the Nav bar shows only 4 days. E.g. the week in the calendar view starts with Sunday -> the Nav bar shows 'Sunday, July 27, 2008 - Thursday, July 31, 2008. This bug is new, the *.xpi which I tested before (Comment #2) shows the correct entry (Sunday - Saturday)
Comment 18•16 years ago
|
||
I also noticed something strange. After cvs upping included this patch, I had the app started and clicked on the "today" button. The 10th (which was today) was selected, but the view header told me I was in *June*. Clicking on "today" again gave me the correct month and I could not further reproduce.
Comment 19•16 years ago
|
||
Once more: Multiweek view, first day is July 6th, last day is August 16th -> Nav bar shows July 06 - August 14th. And in the month view only the navigation buttons are visible in the nav bar.
Assignee | ||
Comment 20•16 years ago
|
||
Obviously my final changes to meet Christian's comment #14 were not so unrisky as I thought, so I in this patch I drew them back. >> Same question here? looks almost like gray, so maybe also a -moz color? >No Christian insists to use his own color definitions. In this case the >font-color has to be adapted to the image color. I disputed this question with Christian some months ago. Christian was basically not happy with the limited offererd color definitions like "-moz-Field" -moz-Dialog" etc. and found they did not suffice his graphical demands. Therfor he accepted that his (hard coded) colors are equal over all themings. Although not being a graphical designer myself I understand that quite well. But in this special issue it makes even more sense to have the color of the font being equal to the color of the arrow images.
Attachment #329044 -
Flags: review?(philipp)
Updated•16 years ago
|
Attachment #329044 -
Flags: review?(philipp) → review+
Assignee | ||
Comment 21•16 years ago
|
||
checked in patch #3 on trunk and MOZILLA_1_8_BRANCH Andreas tested the patch before I gave it to review.
Updated•16 years ago
|
Target Milestone: --- → 0.9
Updated•16 years ago
|
Attachment #328857 -
Attachment description: revised patch #2 → [checked in] revised patch #2
Updated•16 years ago
|
Attachment #329044 -
Attachment description: patch v. #3 → [checked in] patch v. #3
Comment 22•16 years ago
|
||
This checkin has caused the following strict warning: Warning: assignment to undeclared variable categories Source File: chrome://calendar/content/calendar-decorated-day-view.xml Line: 72
Comment 23•16 years ago
|
||
And the button icons are (still) missing in all four views using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.17pre) Gecko/2008071106 Calendar/0.9pre.
Comment 25•16 years ago
|
||
In month view please restore the useful header. I want to see with just one look whether it's "July" or "December". Currently it states "7/1/2008 - 7/31/2008" or "12/1/2008 - 12/31/2008". Regression: Especially when using the long date format this increases the minimum width required for the view very much. This means Sunbird/Lightning can't display correctly anymore on smaller screen resolutions. This regresses all bugs that have been fixed in the past to allow the view to be displayed on smaller screen resolutions.
Comment 26•16 years ago
|
||
This patch includes the missing file chrome://calendar/skin/widgets/calendar-widgets.css that define the button icons in Sunbird.
Attachment #329213 -
Flags: review?(philipp)
Updated•16 years ago
|
Attachment #329213 -
Flags: review?(philipp) → review+
Updated•16 years ago
|
Attachment #329213 -
Attachment description: show button icons in Sunbird too → [checked in] show button icons in Sunbird too
Comment 27•16 years ago
|
||
(In reply to comment #25) > In month view please restore the useful header. I want to see with just one > look whether it's "July" or "December". Currently it states "7/1/2008 - > 7/31/2008" or "12/1/2008 - 12/31/2008". I agree, the month view should say the month name instead of the date. > > Regression: > Especially when using the long date format this increases the minimum width > required for the view very much. This means Sunbird/Lightning can't display > correctly anymore on smaller screen resolutions. This regresses all bugs that > have been fixed in the past to allow the view to be displayed on smaller screen > resolutions. I'd suggest to contract "Calendar Week" to "CW" when the window becomes small. Is this enough? Maybe the inverval could be wrapped into two lines when the window becomes small?
Comment 28•16 years ago
|
||
The Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.0.2pre) Gecko/2008071121 Calendar/0.6a1 nightly build is displaying the month and multiweek view correctly.
Comment 29•16 years ago
|
||
Note: in multi-week view, the weeks are arranged vertically (one, next, next, etc.) -- the buttons for next/prev, however, are horizontal. I wonder if there's a way to have vertically arranged buttons.
Assignee | ||
Comment 30•16 years ago
|
||
patch that introduces the new strings that Christian demanded in comment #9 for the date-formatted display
Attachment #329678 -
Flags: review?(philipp)
Updated•16 years ago
|
Attachment #329678 -
Flags: review?(philipp) → review+
Comment 31•16 years ago
|
||
Comment on attachment 329678 [details] [diff] [review] [checked in] patch with new Strings I think you need a real localization note for each property that clearly describes each parameter and that is recognized by the l10n tools. <http://developer.mozilla.org/en/docs/Localization_notes>
Assignee | ||
Comment 32•16 years ago
|
||
in reply to comment #31: Thank you for that hint. Talked to Simon and will release a dev not in the localization board 'mozilla.dev.l10n' I hope the localization notes in the patch attached give useful assistance to the localizers
Attachment #329793 -
Flags: review?(daniel.boelzle)
Updated•16 years ago
|
Attachment #329678 -
Attachment description: patch with new Strings → [checked in] patch with new Strings
Updated•16 years ago
|
Attachment #328640 -
Attachment is obsolete: true
Updated•16 years ago
|
Attachment #329793 -
Flags: review?(daniel.boelzle) → review+
Assignee | ||
Updated•16 years ago
|
Attachment #329793 -
Attachment description: patch with localization notes → [checked in] patch with localization notes
Assignee | ||
Comment 33•16 years ago
|
||
"patch with localization notes" checked in on trunk and MOZILLA_1_8_BRANCH
Comment 34•16 years ago
|
||
The user gets no horizontal scroll bar in the calendar view since the new Nav Bar is implemented.
Assignee | ||
Comment 35•16 years ago
|
||
patch with the new header with the formats that Christian had in mind in comment. I did not follow his suggestions for the day view: > EN: March 3, 2008 > DE: 3. März 2008 In this case I took what the current standard dateformatter delivered which was slightly different. I also eliminated the strict warning that Stefan discovered in comment #22. I also hope to to meet Andreas' and Stefan's complaints in comment #25 and comment #34 One implementation detail: I added a temporary function "formatNewInterval" to calIDateFormatter.idl and hope to replace the exsting "formatInterval" shortly after this patch has been integrated. - the Before I am asking for review I would like to have this tested by somebody else, like Andreas.
Assignee | ||
Comment 36•16 years ago
|
||
Comment on attachment 329844 [details] [diff] [review] patch with new header format Andreas tested the patch and found it was Ok. So I am asking for review. @philipp: I hope that you can live with my implementation within the dateFormatter.js that is sort of "have finished". I rather want to complete this job within Bug 413103 shortly after.
Attachment #329844 -
Flags: ui-review?(christian.jansen)
Attachment #329844 -
Flags: review?(philipp)
Comment 37•16 years ago
|
||
Comment on attachment 329844 [details] [diff] [review] patch with new header format ui=christian One small nit. It would be good if the day info could be formatted like specified in comment #4. However, this is not blocking.
Attachment #329844 -
Flags: ui-review?(christian.jansen) → ui-review+
Comment 38•16 years ago
|
||
I've set Monday as first day of the week but in multiweek view the header show Sunday as first day (week from Sunday to Saturday instead of Monday to Sunday). Lightning 0.9pre 2008071619
Comment 39•16 years ago
|
||
Comment on attachment 329844 [details] [diff] [review] patch with new header format >+ <property name="rangeStartDate"> >+ <setter><![CDATA[ >+ this.mRangeStartDate = val; >+ return val; If you like, this can be shortened to: return (this.mRangeStartDate = val); r=philipp
Attachment #329844 -
Flags: review?(philipp) → review+
Assignee | ||
Comment 40•16 years ago
|
||
patch checked in on trunk and MOZILLA_1_8_BRANCH. -> fixed I will continue with bug 413103 as promised in comment #36
Assignee | ||
Updated•16 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Comment 41•16 years ago
|
||
The dates shown in the nav bar are still wrong for he multiweek view as pointed out in Comment #19 . I think it's a serious bug and I believe this should be fixed before 0.9 comes out. Worth blocking 0.9?
Comment 42•16 years ago
|
||
(In reply to comment #41) > The dates shown in the nav bar are still wrong for he multiweek view as pointed > out in Comment #19 . I think it's a serious bug and I believe this should be > fixed before 0.9 comes out. Worth blocking 0.9? > It's bug #446366 now.
Comment 43•16 years ago
|
||
Checked in nightly build 2008072219 ->VERIFIED (except the follow-up bug 446366)
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•