Hold on. This *isn't* a check menu item, so the guidline doesn't
apply, right ?

But, in any case - this patch didn't start out using a check menu
item, but I didn't think it worked very well. i.e. you had

  Remove From Panel
  Locked
  Move

and then when locked

  Remove From Panel
x Locked
  Move

I just can't think of a good term to convey the concept as a "state"
rather than a "command".

Any ideas anyone ?

 

> Hold on. This *isn't* a check menu item, so the guidline doesn't
> apply, right ?

If it walks like a duck, quacks like a duck, then it a duck. ;-)  It's
a menu item that toggles between two different states which is a check
menu item.  I'd value calum's feedback on this, too.

Its a mutable command item right now, but a checkbox is probably more
appropriate.

Seth: I've tried using a check menu item but didn't feel it worked
very well. Could you comment on above and maybe suggest better
terminology?

Agree with the general feeling here... since "Lock" functions as both
a verb and a noun, it doesn't make for a very good menu item, mutable
or otherwise.  I agree a checkbox item called "Locked" is even worse
though, since as Mark says, that makes it unclear whether it's a
checkbox item indication the item isn't locked, or a mutable item
indicating it *is* locked.

If this property only affects an applet's movability and not any of
its preferences, perhaps making it a checkbox menu item labelled
"Movable" would be the most accurate solution, although that might
look a little peculiar right next to the "Move" command.  Perhaps the
docs guys have a better suggestion.

FWIW, on Windows the menu item for locking things is a bit of a mess
too IMHO... it's labelled "Lock the <object>" which makes it sound
like a command, whereas in fact it's a checkbox menu item.  Let's do
better :)

I think I've heard Lock to Panel/Unlock suggested somewhere as well
... it makes it more obvious that "Lock" is a command ...

How about instead of "Locked", "Lock in place"?

Or "Lock Position", "Unlock Position"?  Although it does affect
removability too, so maybe that's not strictly accurate either...

I support Mark's suggestion of "Lock to Panel" on the following
grounds: 

- Provides a clear command
- Uses the standard imperative style common to the GNOME Desktop
- Makes the locking parameter clear, i.e. the panel
- Eliminates ambiguity

Also, I support the earlier remark that this is a toggle situation
between two states, so you only need to have a checkbox with the label
"Lock to Panel" cleared or ticked. There is no need for the term
"Unlocked" to appear in the label.

Pat

Hmm, on the checkbox point, I know I'd find it pretty confusing if I
popped up a menu that said "Lccked to Panel" on it when it clearly
wasn't, but maybe I'm just a simple bloke who takes thing too
literally... :)

The checkbox wouldn't say "Locked to Panel" the checkbox would say
"Lock to Panel": 

o Lock to Panel: the checkbox is clear, therefore the label is an
invitation to choose this option if you want to lock the applet to the
panel. 

x Lock to Panel: the checkbox is ticked, therefore you have accepted
the invitation offered by the label to lock the applet to the panel. 

This is exactly the same logic followed elsewhere in the GNOME Desktop
and applications.

Pat

Oops, quite right, a pox on Solaris 8 and its illegible Mozilla fonts...

bug 132967 has some followup discussion on this, which never made it into CVS. 
I would recommend that the suggestion from comment #11 be commited.

Okay, we need a patch for

  [x] Lock to Panel

*** Bug 132967 has been marked as a duplicate of this bug. ***

Created attachment 28281 [details] [review]
Patch to add a checkmenu item "Lock To Panel"

This Patch adds a check menu item,
[ ] Lock To Panel
It makes the "Remove From Panel" menu item to be insensitive, when the panel is
locked.

It must be possible to remove a Locked applet. 
The below patch adds a checkmenu item as well as resolves that bug.. 
ie The "Remove From Panel" menu item sensitivity is not altered. 

Created attachment 28283 [details] [review]
Patch to add a checkmenu item "Lock To Panel" and not to alter the sensitivity of the "Remove From Panel" menuitem

Comments on the patch:

  1) I've already committed stuff to HEAD (earlier today) to allowing
     removing locked applets

  2) Follow the coding style of the rest of the code - i.e.

  gtk_check_menu_item_set_active (checkmenuitem, locked);

     rather than:

  gtk_check_menu_item_set_active(checkmenuitem,locked);

  3) You should be doing something like:

  panel_applet_set_locked (info, gtk_menu_item_get_active (...));

     rather than relying on the state of the applet to stay in sync
     with the menu item.

  4) You're completely ignoring whether the applet_id_list key is writable:

 	bonobo_ui_component_set_prop (frame->priv->ui_component,
-				      "/commands/RemoveAppletFromPanel",
-				      "sensitive",
-				      locked ? "0" : (removable ? "1" : "0"),
-				      NULL);

     the removable flag is there for a reason !

   5) How is this a check menu item:

+         <menuitem name="lock" verb="..." _label="..."/>

      ?

Created attachment 28379 [details] [review]
Revised Patch

Mark: I have changed the patch and i have followed the steps you have
mentioned. Thanks..

Mark: Any comment on the revised patch. Have sticked to the rules and style of coding. 

Created attachment 34163 [details] [review]
Revised Patch to commit against the CVS HEAD

Just attached the revised patch to commit against the recent source

Created attachment 34556 [details] [review]
Revised Patch to commit against the CVS HEAD with comments

Madhan: thanks for the patch.
I committed a slightly modified version.