Re: [Libreoffice] [PATCH] Introduce HideDisabledMenuItems style setting

2011-04-28 Thread Lubos Lunak
On Wednesday 27 of April 2011, Christian Dywan wrote:
 Am 27.04.2011 13:01:36 schrieb Christoph Noack:
  Just a small question ... do I get it right that the disabled menu items
  get hidden instead of being grayed out? Real interest: could you
  please explain what the rationale for the change is? As far as I know,
  there is currently no platform guideline that requires that (or even
  allows that) - but I might be wrong.

 Yes, disabled becomes hidden effectively. That is *current* behaviour on
 all systems as far as I can tell, so I'm not introducing anything new, to
 make that clear. I explicitly want to change this for GTK+ because it is
 wrong behaviour.

 As far as I know, OS X and KDE are the only platforms doing this (I'll
 ignore Maemo). I don't know the exact guideline, I didn't find clear
 documentation about it but for example the web page context menu on OS X
 does it and editor popups I saw in KDE. That said, Lubos or anyone else,
 please correct me if my observation is wrong, I'll adapt the patch
 accordingly.

 I'm not aware of KDE hidding any disabled menu items anywhere, AFAIK they 
always get just greyed out. I don't have any access to OS X, so I don't know 
there, but a google query turned up [*], which also seems to just do that 
instead of hiding.

[*] http://macintopics.com/media/2010/06/context-menu-text-substitutions.jpg

-- 
 Lubos Lunak
 l.lu...@suse.cz
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] [PATCH] Introduce HideDisabledMenuItems style setting

2011-04-28 Thread Lubos Lunak
On Wednesday 27 of April 2011, Christian Lohmaier wrote:
 Hi Christian, *,

 On Thu, Apr 21, 2011 at 3:44 PM, Christian Dywan christ...@lanedo.com 
wrote:
  I'm introducing a setting that decides if disabled menu items should be
  hidden. Currently the code is broken in the sense that items are hidden
  if disabled on all platforms and UpdateApplicationSettings which
  theortically does that is a) counter-intuitive and b) not set by
  platforms.

 I'm not sure I understand.
 I absolutely would hate it when the disabled menu-items were hidden
 instead of just greyed out.

 Why would you want to have that dependent on the Desktop-environment?
 Either the user wants to hide them (but I doubt they really do want
 that), or the user doesn't want it (like me).

 So why should LO behave differently on KDE vs Gnome vs Windows in this
 regard?

 Because platform integration means LO should act like other native 
applications of that platform (as much as possible and reasonable). If e.g. 
KDE popups act in a certain way, then KDE user will certainly prefer if LO 
popups acted the same way, and if Windows 8 decides that all icons should be 
pink on green, then probably LO on W8 should paint icons that way too.

-- 
 Lubos Lunak
 l.lu...@suse.cz
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] [PATCH] Introduce HideDisabledMenuItems style setting

2011-04-27 Thread Christian Dywan
Am 27.04.2011 13:01:36 schrieb Christoph Noack:
 Just a small question ... do I get it right that the disabled menu items 
 get hidden instead of being grayed out? Real interest: could you 
 please explain what the rationale for the change is? As far as I know, 
 there is currently no platform guideline that requires that (or even 
 allows that) - but I might be wrong. 

Yes, disabled becomes hidden effectively. That is *current* behaviour on all 
systems as far as I can tell, so I'm not introducing anything new, to make that 
clear.
I explicitly want to change this for GTK+ because it is wrong behaviour.

As far as I know, OS X and KDE are the only platforms doing this (I'll ignore 
Maemo). I don't know the exact guideline, I didn't find clear documentation 
about it but for example the web page context menu on OS X does it and editor 
popups I saw in KDE.
That said, Lubos or anyone else, please correct me if my observation is wrong, 
I'll adapt the patch accordingly.

I updated the patch and attached a second one to get ird of 
UpdateApplicationSettings which for no good reason tries to flip the same 
switch in a very misleading way.

ciao,
ChristianFrom e6d2363785af7e8021afe358be66db233d360fc4 Mon Sep 17 00:00:00 2001
From: Christian Dywan christian.dy...@lanedo.com
Date: Thu, 21 Apr 2011 15:14:32 +0200
Subject: [PATCH] Introduce HideDisabledMenuItems style setting

The STYLE_OPTION_HIDEDISABLED flag is removed.

The setting is enabled for KDE and OSX.
---
 svtools/source/edit/svmedit.cxx  |3 ---
 vcl/aqua/source/window/salframe.cxx  |1 +
 vcl/inc/vcl/settings.hxx |6 +-
 vcl/source/app/settings.cxx  |3 +++
 vcl/source/control/edit.cxx  |7 ---
 vcl/unx/kde/salnativewidgets-kde.cxx |1 +
 vcl/unx/kde4/KDESalFrame.cxx |1 +
 7 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/svtools/source/edit/svmedit.cxx b/svtools/source/edit/svmedit.cxx
index b182446..b1f8919 100644
--- a/svtools/source/edit/svmedit.cxx
+++ b/svtools/source/edit/svmedit.cxx
@@ -859,9 +859,6 @@ void TextWindow::Command( const CommandEvent rCEvt )
 if ( rCEvt.GetCommand() == COMMAND_CONTEXTMENU )
 {
 PopupMenu* pPopup = Edit::CreatePopupMenu();
-const StyleSettings rStyleSettings = GetSettings().GetStyleSettings();
-if ( rStyleSettings.GetOptions()  STYLE_OPTION_HIDEDISABLED )
-pPopup-SetMenuFlags( MENU_FLAG_HIDEDISABLEDENTRIES );
 if ( !mpExtTextView-HasSelection() )
 {
 pPopup-EnableItem( SV_MENU_EDIT_CUT, sal_False );
diff --git a/vcl/aqua/source/window/salframe.cxx b/vcl/aqua/source/window/salframe.cxx
index f37182f..4b41148 100644
--- a/vcl/aqua/source/window/salframe.cxx
+++ b/vcl/aqua/source/window/salframe.cxx
@@ -1320,6 +1320,7 @@ void AquaSalFrame::UpdateSettings( AllSettings rSettings )
 
 // images in menus false for MacOSX
 aStyleSettings.SetPreferredUseImagesInMenus( false );
+aStyleSettings.SetHideDisabledMenuItems( sal_True );
 aStyleSettings.SetAcceleratorsInContextMenus( sal_False );
 
 rSettings.SetStyleSettings( aStyleSettings );
diff --git a/vcl/inc/vcl/settings.hxx b/vcl/inc/vcl/settings.hxx
index e5a824c..4f16997 100644
--- a/vcl/inc/vcl/settings.hxx
+++ b/vcl/inc/vcl/settings.hxx
@@ -434,6 +434,7 @@ private:
 sal_uLong   mnSymbolsStyle;
 sal_uLong   mnPreferredSymbolsStyle;
 sal_uInt16  mnSkipDisabledInMenus;
+sal_BoolmbHideDisabledMenuItems;
 sal_BoolmnAcceleratorsInContextMenus;
 Wallpaper   maWorkspaceGradient;
 const void* mpFontOptions;
@@ -456,7 +457,6 @@ private:
 #define STYLE_OPTION_SPINARROW  ((sal_uLong)0x0080)
 #define STYLE_OPTION_SPINUPDOWN ((sal_uLong)0x0100)
 #define STYLE_OPTION_NOMNEMONICS((sal_uLong)0x0200)
-#define STYLE_OPTION_HIDEDISABLED   ((sal_uLong)0x0010)
 
 #define DRAGFULL_OPTION_WINDOWMOVE  ((sal_uLong)0x0001)
 #define DRAGFULL_OPTION_WINDOWSIZE  ((sal_uLong)0x0002)
@@ -736,6 +736,10 @@ public:
 { CopyData(); mpData-mnSkipDisabledInMenus = bSkipDisabledInMenus; }
 sal_Bool			GetSkipDisabledInMenus() const
 { return (sal_Bool) mpData-mnSkipDisabledInMenus; }
+void   SetHideDisabledMenuItems( sal_Bool bHideDisabledMenuItems )
+{ CopyData(); mpData-mbHideDisabledMenuItems = bHideDisabledMenuItems; }
+sal_Bool			GetHideDisabledMenuItems() const
+{ return mpData-mbHideDisabledMenuItems; }
 void   SetAcceleratorsInContextMenus( sal_Bool bAcceleratorsInContextMenus )

Re: [Libreoffice] [PATCH] Introduce HideDisabledMenuItems style setting

2011-04-27 Thread Christian Lohmaier
Hi Christian, *,

On Thu, Apr 21, 2011 at 3:44 PM, Christian Dywan christ...@lanedo.com wrote:
 I'm introducing a setting that decides if disabled menu items should be 
 hidden. Currently the code is broken in the sense that items are hidden if 
 disabled on all platforms and UpdateApplicationSettings which theortically 
 does that is a) counter-intuitive and b) not set by platforms.

I'm not sure I understand.
I absolutely would hate it when the disabled menu-items were hidden
instead of just greyed out.

Why would you want to have that dependent on the Desktop-environment?
Either the user wants to hide them (but I doubt they really do want
that), or the user doesn't want it (like me).

So why should LO behave differently on KDE vs Gnome vs Windows in this regard?

 The patch sets the setting for KDE, KDE4 and OSX and uses it for the edit 
 menu.

what effect is setting this setting - I don't understand. You mean
it defaults to hide them instead of greying them out for KDE* and OSX?

And I guess uses it for the edit menu means that your patch limits
its scope to one menu only for a proof-of-concept?

 Future steps are to use the setting for other menus and I think 
 UpdateApplicationSettings should eventually be removed because it's 
 completely mis-designed and confusing.

Well, not sure what your intention is, as I didn't fully understand
what you were trying to say, but you sure got my veto when hiding
the menus will be default/cannot be turned off.

ciao
Christian
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] [PATCH] Introduce HideDisabledMenuItems style setting

2011-04-27 Thread Christoph Noack
Hi Christian!

Sorry, it seems that I'm still a bit confused about that issue ...

Am Mittwoch, den 27.04.2011, 11:13 + schrieb Christian Dywan:
 Am 27.04.2011 13:01:36 schrieb Christoph Noack:
  Just a small question ... do I get it right that the disabled menu items 
  get hidden instead of being grayed out? Real interest: could you 
  please explain what the rationale for the change is? As far as I know, 
  there is currently no platform guideline that requires that (or even 
  allows that) - but I might be wrong. 
 
 Yes, disabled becomes hidden effectively. That is *current* behaviour
 on all systems as far as I can tell, so I'm not introducing anything
 new, to make that clear.

Okay, then we really talk about those items in the application menu -
now being hidden instead of grayed out temporarily (during runtime).

 I explicitly want to change this for GTK+ because it is wrong
 behaviour.
 
 As far as I know, OS X and KDE are the only platforms doing this (I'll
 ignore Maemo). I don't know the exact guideline, I didn't find clear
 documentation 

Mac OS X Human Interface Guidelines
Naming Menu Items: When a menu item is unavailable—because it
doesn’t apply to the selected object or to the selected object
in its current state, or because nothing is selected, for
example—the item should appear dimmed (gray) in the menu ...

http://developer.apple.com/library/mac/#documentation/UserExperience/Conceptual/AppleHIGuidelines/XHIGMenus/XHIGMenus.html#//apple_ref/doc/uid/TP3356-TP6

KDE User Interface Guidelines
Menu items should not be added or removed during runtime.
Disable or enable them instead.

http://developer.kde.org/documentation/standards/kde/style/menus/index.html

GNOME Human Interface Guidelines
4.3.2.1. Command Items Guidelines: Do not remove command items
from the menu when they are unavailable, make them insensitive
instead.
http://developer.gnome.org/hig-book/2.91/menus-design.html.en

Microsoft User Experience Interaction Guidelines
Disable menu items that don't apply to the current context,
instead of removing them. Doing so makes menu bar contents
stable and easier to find.
http://msdn.microsoft.com/en-us/library/aa511502.aspx#presentation


Is that the information you searched for?


 about it but for example the web page context menu on OS X does it and
 editor popups I saw in KDE.

In this case, context menus are a different matter - they hide elements
instead of removing them (or positively: only show what's available).

But we have other issues within our context menus that need to be fixed
(since 15 years or so *g*).

 That said, Lubos or anyone else, please correct me if my observation
 is wrong, I'll adapt the patch accordingly.

[...]

Regards,
Christoph

-- 
LibreOffice Design Team. Make it just work, and look great, too!
http://wiki.documentfoundation.org/Design

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


[Libreoffice] [PATCH] Introduce HideDisabledMenuItems style setting

2011-04-21 Thread Christian Dywan
I'm introducing a setting that decides if disabled menu items should be hidden. 
Currently the code is broken in the sense that items are hidden if disabled on 
all platforms and UpdateApplicationSettings which theortically does that is a) 
counter-intuitive and b) not set by platforms.
The patch sets the setting for KDE, KDE4 and OSX and uses it for the edit menu.

Future steps are to use the setting for other menus and I think 
UpdateApplicationSettings should eventually be removed because it's completely 
mis-designed and confusing.

ciao,
ChristianFrom 3475f8f442ffed53df728acfd0e96b2bab1d18cd Mon Sep 17 00:00:00 2001
From: Christian Dywan christian.dy...@lanedo.com
Date: Thu, 21 Apr 2011 15:14:32 +0200
Subject: [PATCH] Introduce HideDisabledMenuItems style setting

The setting is enabled for KDE and OSX.
---
 vcl/aqua/source/window/salframe.cxx  |1 +
 vcl/inc/vcl/settings.hxx |5 +
 vcl/source/app/settings.cxx  |3 +++
 vcl/source/control/edit.cxx  |4 +++-
 vcl/unx/kde/salnativewidgets-kde.cxx |1 +
 vcl/unx/kde4/KDESalFrame.cxx |1 +
 6 files changed, 14 insertions(+), 1 deletions(-)

diff --git a/vcl/aqua/source/window/salframe.cxx b/vcl/aqua/source/window/salframe.cxx
index f37182f..4b41148 100644
--- a/vcl/aqua/source/window/salframe.cxx
+++ b/vcl/aqua/source/window/salframe.cxx
@@ -1320,6 +1320,7 @@ void AquaSalFrame::UpdateSettings( AllSettings rSettings )
 
 // images in menus false for MacOSX
 aStyleSettings.SetPreferredUseImagesInMenus( false );
+aStyleSettings.SetHideDisabledMenuItems( sal_True );
 aStyleSettings.SetAcceleratorsInContextMenus( sal_False );
 
 rSettings.SetStyleSettings( aStyleSettings );
diff --git a/vcl/inc/vcl/settings.hxx b/vcl/inc/vcl/settings.hxx
index e5a824c..a6a9565 100644
--- a/vcl/inc/vcl/settings.hxx
+++ b/vcl/inc/vcl/settings.hxx
@@ -434,6 +434,7 @@ private:
 sal_uLong   mnSymbolsStyle;
 sal_uLong   mnPreferredSymbolsStyle;
 sal_uInt16  mnSkipDisabledInMenus;
+sal_BoolmbHideDisabledMenuItems;
 sal_BoolmnAcceleratorsInContextMenus;
 Wallpaper   maWorkspaceGradient;
 const void* mpFontOptions;
@@ -736,6 +737,10 @@ public:
 { CopyData(); mpData-mnSkipDisabledInMenus = bSkipDisabledInMenus; }
 sal_Bool			GetSkipDisabledInMenus() const
 { return (sal_Bool) mpData-mnSkipDisabledInMenus; }
+void   SetHideDisabledMenuItems( sal_Bool bHideDisabledMenuItems )
+{ CopyData(); mpData-mbHideDisabledMenuItems = bHideDisabledMenuItems; }
+sal_Bool			GetHideDisabledMenuItems() const
+{ return mpData-mbHideDisabledMenuItems; }
 void   SetAcceleratorsInContextMenus( sal_Bool bAcceleratorsInContextMenus )
 { CopyData(); mpData-mnAcceleratorsInContextMenus = bAcceleratorsInContextMenus; }
 sal_Bool			GetAcceleratorsInContextMenus() const
diff --git a/vcl/source/app/settings.cxx b/vcl/source/app/settings.cxx
index b256f04..388a043 100644
--- a/vcl/source/app/settings.cxx
+++ b/vcl/source/app/settings.cxx
@@ -529,6 +529,7 @@ ImplStyleData::ImplStyleData( const ImplStyleData rData ) :
 mnUseImagesInMenus			= rData.mnUseImagesInMenus;
 mbPreferredUseImagesInMenus	= rData.mbPreferredUseImagesInMenus;
 mnSkipDisabledInMenus		= rData.mnSkipDisabledInMenus;
+mbHideDisabledMenuItems  = rData.mbHideDisabledMenuItems;
 mnAcceleratorsInContextMenus	= rData.mnAcceleratorsInContextMenus;
 mnToolbarIconSize			= rData.mnToolbarIconSize;
 mnSymbolsStyle= rData.mnSymbolsStyle;
@@ -618,6 +619,7 @@ void ImplStyleData::SetStandardStyles()
 mnUseFlatMenues 			= 0;
 mbPreferredUseImagesInMenus	= sal_True;
 mnSkipDisabledInMenus		= (sal_uInt16)sal_False;
+mbHideDisabledMenuItems		= sal_False;
 mnAcceleratorsInContextMenus	= sal_True;
 
 Gradient aGrad( GRADIENT_LINEAR, DEFAULT_WORKSPACE_GRADIENT_START_COLOR, DEFAULT_WORKSPACE_GRADIENT_END_COLOR );
@@ -1079,6 +1081,7 @@ sal_Bool StyleSettings::operator ==( const StyleSettings rSet ) const
  (mpData-mnUseImagesInMenus		== rSet.mpData-mnUseImagesInMenus)			
  (mpData-mbPreferredUseImagesInMenus == rSet.mpData-mbPreferredUseImagesInMenus) 
  (mpData-mnSkipDisabledInMenus		== rSet.mpData-mnSkipDisabledInMenus)		
+ (mpData-mbHideDisabledMenuItems	== rSet.mpData-mbHideDisabledMenuItems)	
  (mpData-mnAcceleratorsInContextMenus	== rSet.mpData-mnAcceleratorsInContextMenus)		
  (mpData-maFontColor== rSet.mpData-maFontColor ))
 

Re: [Libreoffice] [PATCH] Introduce HideDisabledMenuItems style setting

2011-04-21 Thread Christoph Noack
Hi Christian, all!

Just a small question ... do I get it right that the disabled menu items
get hidden instead of being grayed out? Real interest: could you
please explain what the rationale for the change is? As far as I know,
there is currently no platform guideline that requires that (or even
allows that) - but I might be wrong.

Cheers,
Christoph

Am Donnerstag, den 21.04.2011, 13:44 + schrieb Christian Dywan:
 I'm introducing a setting that decides if disabled menu items should
 be hidden. Currently the code is broken in the sense that items are
 hidden if disabled on all platforms and UpdateApplicationSettings
 which theortically does that is a) counter-intuitive and b) not set by
 platforms.
 The patch sets the setting for KDE, KDE4 and OSX and uses it for the
 edit menu.
 
 Future steps are to use the setting for other menus and I think
 UpdateApplicationSettings should eventually be removed because it's
 completely mis-designed and confusing.
 
 ciao,
 Christian

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice