275695 - Rework Sunbird menubar and menus

Status: VERIFIED FIXED

(Calendar :: Sunbird Only, defect)

Description

[Matthew (lilmatt) Willis]

The Sunbird menubar needs work.
We should add things like Undo and Redo, and being a toolkit app, the Extension
and Theme Managers.

Comment 1

[Matthew (lilmatt) Willis]

Attachment: rev0 - first cut at the menu changes

Comment 2

[Matthew (lilmatt) Willis]

From IRC:
11:51 <lilmatt> So is it okay for me to begin landing my menubar changes as 
                described in the wiki on the trunk?
12:40 <stuart> i'm all for landing whatever menu stuff

Patch checked in.

Comment 3

[Mostafa Hosseini]

For reference, here's the section in the wiki that discusses the change:
http://wiki.mozilla.org/index.php/Calendar:Sunbird_UIReview

Also, the usual process would be to ask [e-mail] for a review and check
the patch in once he grants the patch a first review. This way it will give
others a chance to comment, seeing that the change is about to land.

Comment 4

[Matthew (lilmatt) Willis]

Sorry. Will do in the future.

For reference, I will make the labels localizable once we've locked the menus
down more tightly.

Comment 5

[gekacheka]

Nit: 

The menu item 
  "View | Workweek days only" 
is listed as
  "View | Only show work days"

The former wording was chosen because "workweek days" makes clear the context of
days within a week.   "Work days" does not give the "week" context, and can be
wrongly taken to exclude holidays as well as weekend days.  (Also "show" is
redundant in the "view" menu.)

Comment 6

[Matthew (lilmatt) Willis]

(In reply to comment #5)
> The former wording was chosen because "workweek days" makes clear the context of
> days within a week.   "Work days" does not give the "week" context, and can be
> wrongly taken to exclude holidays as well as weekend days.  (Also "show" is
> redundant in the "view" menu.)

Thanks. I'll revert it in the next patch.

Comment 7

[Matthew (lilmatt) Willis]

Attachment: rev1 - trunk - incorporates gekacheka's comment and other minor changes

Comment 8

[Matthew (lilmatt) Willis]

(In reply to comment #7)
> Created an attachment (id=169852) [edit]
> rev1 - trunk - incorporates gekacheka's comment and other minor changes
 
Checked in.

Comment 9

[Joey Minta]

Matt,
   Are you still planning to work on this one?  It's an 0.3a1 blocker (at least
to get the sunbird menu to look like cvs's calendar.xul).  If not, we should get
this set to [e-mail], so somebody can finish this.

Comment 10

[Matthew (lilmatt) Willis]

I likely won't be able to get to this bug this month.

Setting back to [e-mail] as requested.

Comment 11

[Simon Paquet [:sipaq]]

(In reply to comment #10)
> I likely won't be able to get to this bug this month.

Matt, was is missing here? If it's just the localization of th currently
hardcoded Menu labels, then I can surely make this happen.

Comment 12

[Joey Minta]

(In reply to comment #11)
> Matt, was is missing here? If it's just the localization of th currently
> hardcoded Menu labels, then I can surely make this happen.

Simon,
   I added it as a blocker mostly because Undo/Redo have the wrong commands
assigned, and hence don't work, but localizing would be great.  Also, some of
the open/new calendar commands are getting changed, so some work might need to
be done there.  (See my patch on bug 300254)

Comment 13

[Joey Minta]

Attachment: activate commands

This should finish activating all the remaining commands (undo/redo/quit and
fix import/export).  It doesn't do anything for l10n, though.  We should decide
how string complete we want to be for 0.3a1, and that will determine if this
still blocks it.

Comment 14

[Michiel van Leeuwen (email: mvl+moz@)]

Comment on attachment 191339 [details] [diff] [review]
activate commands

>+           <menuitem id="calendar-quit-menu"
>+                     label="Quit"
>+                     oncommand="window.close()"/>

I think that won't really quit the app if you have a child windo open, like a
event dialog. I don't really know how to make that work correctly though. I
need to check the source from other apps.

Comment 15

[Joey Minta]

Attachment: activate commands v2

Toolkit (and xpfe) includes a nice goQuitApplication() in globalOverlay.js. 
This version uses it (like firefox does) and doesn't have the import-export
stuff that was in the previous version, since that got checked in earlier.

Comment 16

[Michiel van Leeuwen (email: mvl+moz@)]

Comment on attachment 191400 [details] [diff] [review]
activate commands v2

looks good. r=mvl

Comment 17

[Joey Minta]

Comment on attachment 191400 [details] [diff] [review]
activate commands v2

Patch checked in.  Anyone should feel free to take the l10n issues that remain.
 A decision should be made on if this still blocks 0.3a1 now.

Comment 18

[Simon Paquet [:sipaq]]

Attachment: Localize hardcoded entities v1

As mentioned in comment 11, here is my take on the l10n part to finally get
this bug from the radar.

Comment 19

[Simon Paquet [:sipaq]]

(In reply to comment #18)
> Created an attachment (id=191796) [edit]
> Localize hardcoded entities v1

In this patch I also took the liberty of adding in some missing id's and
accesskeys as I came across them. Since my build machine is offline at the
moment (graphics card seems to be screwed), I couldn't test this properly.

Comment 20

[Joey Minta]

Comment on attachment 191796 [details] [diff] [review]
Localize hardcoded entities v1

Fantastic.  Tested on linux with a clean build and everything worked great.

Leave the bug open though, since there's still a few things that need to be
cleaned up (localize 'Quit' and fix 'Select All').

r=jminta

Comment 21

[Simon Paquet [:sipaq]]

Attachment: Localize hardcoded entities v2

>Leave the bug open though, since there's still a few things that need to be
>cleaned up (localize 'Quit' and fix 'Select All').

I fixed the "quit"-part (and also the key codes for "new journal", "undo" and
"redo" in calendar-sets.inc). I don't know what you mean by fixing "select
all", so I couldn't do anything about that.

Comment 22

[Joey Minta]

Comment on attachment 191825 [details] [diff] [review]
Localize hardcoded entities v2

Looks good.  I'm fixing the  'Select All' as we speak.	(It still uses the old
gEventSource.)

Comment 23

[Joey Minta]

Attachment: fix 'Select All'

Other than 'Print', which is a much larger task for a separate bug, 'Select
All' is the final menu-item that worked in 0.2, but doesn't work in trunk. 
This updates it.

'Select All' now selects all events in the current view, which seemed like the
intuitively correct behavior (rather than selecting every event ever created).

This should finally finish the bug.

Comment 24

[Alex Vincent [:WeirdAl]]

Attachment: fix DTD bustage

A bunch of > characters were missed with a checkin on this bug.

Comment 25

[Joey Minta]

Comment on attachment 191854 [details] [diff] [review]
fix DTD bustage

Thanks for the patch.  Checked in.

r=jminta (who sucks as a reviewer)

Comment 26

[Joey Minta]

Attachment: fix menubar bustage

Missing a quote in calendara-menubar.inc

Comment 27

[Robin Edrenius]

Attachment: Fix 'show status bar'

Comment 28

[Joey Minta]

Comment on attachment 192405 [details] [diff] [review]
Fix 'show status bar'

Thanks for the patch.  Checked in.

Comment 29

[Joey Minta]

'Select All' patch checked in.  Leaving this open to fix the parts depending on
bug 195580.  However, this bug no longer blocks 0.3a1.

Comment 30

[Robin Edrenius]

Attachment: Localize hardcoded entry

Localizing a hardcoded entry, I guess this is the right place to attach the
patch.

Comment 31

[Robin Edrenius]

Comment on attachment 194086 [details] [diff] [review]
Localize hardcoded entry

Sorry, posted in wrong bug

Comment 32

[Ross M. W. Bennetts]

(In reply to comment #23)
> Created an attachment (id=191829) [edit]
> fix 'Select All'
> 
> Other than 'Print', which is a much larger task for a separate bug, 'Select
> All' is the final menu-item that worked in 0.2, but doesn't work in trunk. 
> This updates it.
> 
> 'Select All' now selects all events in the current view, which seemed like the
> intuitively correct behavior (rather than selecting every event ever created).
> 
> This should finally finish the bug.


This doesn't seem to work if you are in the 'List' view.
It still only selects the current month/week/day as selected in the lower viewing area.
As such there is now no way of selecting an entire years worth of events and exporting them, other than to select each event individually within the 'List' view.

Thanks...
   Ross...

Comment 33

[Stefan Sitter [:ssitter]]

(In reply to comment #32)
> This doesn't seem to work if you are in the 'List' view.
That is Bug 321384.

I'm resolving this bug as fixed because the patches have been checked in long time ago. Further work can be done in follow up bugs, e.g. bug 327912.