Closed Bug 291105 Opened 15 years ago Closed 14 years ago

ChatZilla's menu structure needs improving

Categories

(Other Applications :: ChatZilla, defect, trivial)

defect
Not set
trivial

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: bugmail, Assigned: Gijs)

References

(Blocks 1 open bug)

Details

(Whiteboard: [cz-0.9.71])

Attachments

(2 obsolete files)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b2) Gecko/20050418 Firefox/1.0+
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b2) Gecko/20050418 Firefox/1.0+

ChatZilla's menus have started to look smushed and atrocious, especially the
File menu. In the *client* tab, every item has its own group. Logical thinking
groups several things together. Print and Save can be grouped together (they
both deal with the view pane), and Join can be grouped with Part and Disconnect
(all deal with the server). In the Edit menu, Select All should be grouped with
Cut, Copy, etc. because they all deal with eachother and the manipulation of text.

Gijs / Hannibal and I also brought up the fact that the server items in the File
menu should be placed somewhere else (who knows where), because they don't deal
with files at all.

I point to the Apple Human Interface Guidelines, which are rather nice. Places
to read:
http://developer.apple.com/documentation/UserExperience/Conceptual/OSXHIGuidelines/XHIGMenus/chapter_7_section_3.html#//apple_ref/doc/uid/TP30000356/TPXREF118
[Grouping Items in Menus]
http://developer.apple.com/documentation/UserExperience/Conceptual/OSXHIGuidelines/XHIGMenus/chapter_7_section_4.html#//apple_ref/doc/uid/TP30000356/TPXREF105
[The File Menu]


Reproducible: Always
My major concern with the menus currently is that some work on the current view
only, and some globally, and it is not consistent at all.

I will point out that there are some items for the file menu that are currently
not visible because they are not implemented yet, so it wont stay [quite] as bad
as it looks.

The View menu needs a good hitting, OTOH.
Status: UNCONFIRMED → NEW
Ever confirmed: true
So, does someone want to have a bash at this? Things that probably need doing...

  - listing all commands that currently have a UI entry point
  - listing all commands, and grouping them sensibly
  - seeing if there are groups where only some commands have UI, and evaluating
if any others should have UI
  - same but for removing UI
  - try to structure the top-level, and context, menus to match the command groups
(In reply to David Corry, comment #0)
> Gijs / Hannibal and I also brought up the fact that the server items in the File
> menu should be placed somewhere else (who knows where), because they don't deal
> with files at all.

How about adding a Server menu (or Network menu) between Tools and Help?

Prog.
Wiki page for menus now sitting at http://wiki.mozilla.org/ChatZilla:Menus -
feel free to move stuff around, add and remove other items, and generally 'fix'
it. :)
Blocks: chatzilla1.0
Attached patch Patch (obsolete) — Splinter Review
We've debated long, hard, and valiantly, and now it's done. And probably imperfect, since I'm so incredibly tired by now. But we have a better, more semantically correct menu structure now. Yay!

r? = silver
Attachment #209529 - Flags: review?(silver)
Comment on attachment 209529 [details] [diff] [review]
Patch

>+    client.menuSpecs["popup:usercommands"] = {
>+        label: MSG_MNU_USERCOMMANDS,
>+        items:
>+        [
>+         ["query",    {visibleif: "cx.user"}],
>+         ["whois",    {visibleif: "cx.user"}],
>+         ["whowas",   {visibleif: "cx.nickname && !cx.user"}],
>+         ["ping",     {visibleif: "cx.user"}],
>+         ["time",     {visibleif: "cx.user"}],
>+         ["version",  {visibleif: "cx.user"}],
>+         ["-",        {visibleif: "cx.user"}],
>+         ["dcc-chat", {visibleif: "cx.user"}],
>+         ["dcc-send", {visibleif: "cx.user"}],
>+        ]
>+    };
>+
>+
>     client.menuSpecs["context:userlist"] = {
>         getContext: getUserlistContext,
>         items:
>         [
>          ["toggle-usort", {type: "checkbox",
>                            checkedif: "client.prefs['sortUsersByMode']"}],
>          ["toggle-umode", {type: "checkbox",
>                            checkedif: "client.prefs['showModeSymbols']"}],
>-         ["-", {visibleif: "cx.nickname"}],
>+         ["-"],
>          ["label-user", {visibleif: "cx.nickname", header: true}],
>-         [">popup:opcommands", {visibleif: "cx.channel && " + isopish + "cx.user"}],
>-         ["whois",   {visibleif: "cx.nickname"}],
>-         ["query",   {visibleif: "cx.nickname"}],
>-         ["version", {visibleif: "cx.nickname"}],
>+         [">popup:opcommands", {enabledif: "cx.channel && " + isopish + "cx.user"}],
>+         [">popup:usercommands", {enabledif: "cx.channel && cx.nickname"}],

You've got some weird visisble/enabled conbinations here.

The two submenus should be visible if the label-user is. The best way (as it did before) is to hide the entire of ["-"], label-user and the two sub-menus when there is no nickname.

I think the enabledif for >popup:opcommands is actually invalid, too.
Attachment #209529 - Flags: review?(silver) → review-
I guess I'll take this.
Assignee: rginda → gijskruitbosch+bugs
Status: NEW → ASSIGNED
So. Hopefully I've fixed the nits. I've certainly tried, to I think I haven't done things entirely by the comments, since I thought that if we don't enforce having a cx.channel, the submenus will show on message contexts on nextwork tabs and such (which is obviously not what we want). I'm not sure about that though, feel free to correct me. The userlist doesn't need those checks, since it never shows users on network or client tabs.

I fixed a few nits of my own, which are:
- on XULRunner, the Open in New Tab and Open in New Window menuitems are inredibly useless, they just throw errors. Showing them on that host makes no sense.
- iAmOp(), iAmHalfOp() and iAmVoice() should never be true if you're not even active in the channel (heck, they shouldn't throw errors in that case which they did now). iAmVoice() should now actually work. Fortunately, nothing uses iAmVoice, which is why it doesn't make a damn difference in the end anyway :-).

Requesting review, again, from Silver.
Attachment #209529 - Attachment is obsolete: true
Attachment #210926 - Flags: review?(silver)
Comment on attachment 210926 [details] [diff] [review]
[checked in] Patch with nits fixed (hopefully) and some nits of my own

r=silver

Those menu conditions are so hard to think through. I think it's all ok, but I guess we'll find out!
Attachment #210926 - Flags: review?(silver) → review+
Attachment #210926 - Attachment description: Patch with nits fixed (hopefully) and some nits of my own → [checked in] Patch with nits fixed (hopefully) and some nits of my own
Attachment #210926 - Attachment is obsolete: true
Checked in --> FIXED.

Any problems or issues with the new layout should be address to new bugs.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Whiteboard: [cz-0.9.71]
You need to log in before you can comment on or make changes to this bug.