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)

defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: giermann, Assigned: giermann)

Details

Attachments

(1 file, 2 obsolete files)

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).
Flags: wanted-calendar0.8?
OS: Windows XP → All
Hardware: PC → All
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attached file Cleanup patch for bug #408798 (obsolete) —
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)
Attached patch Cleanup patch for bug #408798 (obsolete) — Splinter Review
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)
Attachment #298938 - Attachment is obsolete: true
Attachment #298938 - Flags: review?(Berend.Cornelius)
Sorry for the spam, I did not get any response to the first try of attaching the patch...
Assignee: nobody → giermann
Status: NEW → ASSIGNED
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"?
Attachment #298939 - Flags: review?(Berend.Cornelius) → review+
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+
Target Milestone: --- → 0.8
patch checked in on trunk and MOZILLA_1_8_BRANCH.
=> issue is fixed
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Flags: wanted-calendar0.8?
Keywords: checkin-needed
Verified with Lightning 2008012900
Status: RESOLVED → VERIFIED
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.