Closed
Bug 413620
Opened 16 years ago
Closed 16 years ago
Unify/cleanup navigation menu in all modes (Mail/Calendar/Task)
Categories
(Calendar :: Lightning Only, defect)
Calendar
Lightning Only
Tracking
(Not tracked)
VERIFIED
FIXED
0.8
People
(Reporter: giermann, Assigned: giermann)
Details
Attachments
(1 file, 2 obsolete files)
17.14 KB,
patch
|
giermann
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.8.1.11) Gecko/20071127 Firefox/2.0.0.11 Build Identifier: Lightning 0.8pre (build 2008012119) The Navigation menu should be unique in all 3 modes to switch between these modes and to view the address book. Reproducible: Always Actual Results: I found the following: Mail mode: - no menu to switch to Calendar/Task mode (only in Mode toolbar) - Addressbook in "Tools" Calendar mode: - Mail/Calendar/Task switching in menu "Navigation" - Addressbook both in "Navigation" and "Tools" Task mode: - Mail/Calendar/Task switching in menu "Navigation" - Addressbook only in "Tools" Expected Results: Find a unique place to switch between modes and to display Addressbook and leave these at the same position for all modes. You may recognize that this is some kind of spin-off bug for attachment #280382 [details] [diff] [review] (bug #386479).
Assignee | ||
Updated•16 years ago
|
Flags: wanted-calendar0.8?
OS: Windows XP → All
Hardware: PC → All
Updated•16 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee | ||
Comment 1•16 years ago
|
||
I tried to clean up the code of attachment #297819 [details] [diff] [review]. I decided to display "Addressbook" both in "Go" and "Tools" as suggested by Christian in [1]. @Berend: Could you please have a look at this? Is there any reason you stored the menus in 'copyPopupMenus()' into 'menulist' but did not use them for creating the calendar/task menu popups? I had also to remove the FIRST menuseparators in 'removeLastMenuSeparator()' to clean up the "Go" menu. [1] http://spreadsheets.google.com/pub?key=plCAueWeXt4hSxoRkxWFJCQ
Attachment #298938 -
Flags: review?(Berend.Cornelius)
Assignee | ||
Comment 2•16 years ago
|
||
I tried to clean up the code of attachment #297819 [details] [diff] [review]. I decided to display "Addressbook" both in "Go" and "Tools" as suggested by Christian in [1]. @Berend: Could you please have a look at this? Is there any reason you stored the menus in 'copyPopupMenus()' into 'menulist' but did not use them for creating the calendar/task menu popups? I had also to remove the FIRST menuseparators in 'removeLastMenuSeparator()' to clean up the "Go" menu. [1] http://spreadsheets.google.com/pub?key=plCAueWeXt4hSxoRkxWFJCQ
Attachment #298939 -
Flags: review?(Berend.Cornelius)
Assignee | ||
Updated•16 years ago
|
Attachment #298938 -
Attachment is obsolete: true
Attachment #298938 -
Flags: review?(Berend.Cornelius)
Assignee | ||
Comment 3•16 years ago
|
||
Sorry for the spam, I did not get any response to the first try of attaching the patch...
Updated•16 years ago
|
Assignee: nobody → giermann
Updated•16 years ago
|
Status: NEW → ASSIGNED
Comment 4•16 years ago
|
||
Your patch works great and leaves nothing to complain. >+ ltnRemoveMailOnlyItems(calendarpopuplist, "calendar"); >+ ltnRemoveMailOnlyItems(taskpopuplist, "task"); Good point to consolidate this! >Is there any reason you stored >the menus in 'copyPopupMenus()' into 'menulist' but did not use them for >creating the calendar/task menu popups? -I found the code more self explaining with document.getElementById("blablaMenu"). Anyway I like your version better now. The best thing would probably be to use constants instead of literals for the array-index. -The function-name "removeLastMenuSeparator" does not really fit anymore. How about "removeNeedlessSeparators"?
Updated•16 years ago
|
Attachment #298939 -
Flags: review?(Berend.Cornelius) → review+
Assignee | ||
Comment 5•16 years ago
|
||
Leaving r=Berend.Corenelius, as only the name of the function changed. -- ready for check-in --
Attachment #298939 -
Attachment is obsolete: true
Attachment #299696 -
Flags: review+
Updated•16 years ago
|
Keywords: checkin-needed
Updated•16 years ago
|
Target Milestone: --- → 0.8
Comment 6•16 years ago
|
||
patch checked in on trunk and MOZILLA_1_8_BRANCH. => issue is fixed
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Updated•16 years ago
|
Flags: wanted-calendar0.8?
Keywords: checkin-needed
Comment 8•16 years ago
|
||
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.9) Gecko/20071031 Lightning/0.8pre (2008012818) Thunderbird/2.0.0.9 ID:2007103104 VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•