Bug 120882

Summary:	"Raise to Top" and "Lower to Bottom" have no effect
Product:	[Applications] kst	Reporter:	Andrew Walker <arwalker>
Component:	general	Assignee:	kst
Status:	RESOLVED FIXED
Severity:	normal
Priority:	NOR
Version:	1.x
Target Milestone:	---
Platform:	Compiled Sources
OS:	Linux
Latest Commit:		Version Fixed In:
Sentry Crash Report:

Description Andrew Walker 2006-01-27 19:26:30 UTC

Version:           HEAD (using KDE KDE 3.5.0)
Installed from:    Compiled From Sources
OS:                Linux

PROBLEM:
There are circumstances when the "Raise to Top" and "Lower to Bottom" menu entries have no effect on the object they pertain to

STEPS TO REPRODUCE:
Start Kst
Create a rectangle view object
Create a second rectangle view object which is completely contained by the first
Right click on the second rectangle
Select the Lower to Bottom menu item

RESULTS:
No effect

EXPECTED RESULTS:
The second rectangle should be lowered in the z-order and be hidden by the first rectangle created

Comment 1 Andrew Walker 2006-01-27 19:29:35 UTC

Alternatively the menu items should be disabled if they are not functional

Comment 2 George Staikos 2006-01-27 22:04:52 UTC

On Friday 27 January 2006 13:29, Andrew Walker wrote:
> ------- Alternatively the menu items should be disabled if they are not
> functional _______________________________________________

  For some reason all my mail to bugs.kde.org is majorly delayed.  Anyway, 
this is the correct approach.  The behavior is right, but the menu items are 
wrong as it currently stands.

Comment 3 George Staikos 2006-01-27 22:09:55 UTC

The second one is a child of the first, so it's already at the bottom.  It 
can't be lowered below its parent.  The only thing I see wrong here is that 
we should disable the lower/raise actions when they're not useful.  That is, 
when the object already is the lowest or highest.

Comment 4 Andrew Walker 2006-01-27 22:50:37 UTC

Proposed patch for this problem:

Index: kstviewobject.cpp
===================================================================
--- kstviewobject.cpp   (revision 503013)
+++ kstviewobject.cpp   (working copy)
@@ -1080,7 +1080,8 @@

   bool rc = false;
   int id;
-
+  int index;
+
   _moveToMap.clear();

   if (!tagName().isEmpty()) {
@@ -1098,23 +1099,35 @@
   }

   if (_layoutActions & Raise) {
-    menu->insertItem(i18n("&Raise"), this, SLOT(raise()));
+    index = menu->insertItem(i18n("&Raise"), this, SLOT(raise()));
     rc = true;
+    if (_parent && _parent->_children.count() == 1) {
+      menu->setItemEnabled(index, false);
+    }
   }

   if (_layoutActions & Lower) {
-    menu->insertItem(i18n("&Lower"), this, SLOT(lower()));
+    index = menu->insertItem(i18n("&Lower"), this, SLOT(lower()));
     rc = true;
+    if (_parent && _parent->_children.count() == 1) {
+      menu->setItemEnabled(index, false);
+    }
   }

   if (_layoutActions & RaiseToTop) {
-    menu->insertItem(i18n("Raise to &Top"), this, SLOT(raiseToTop()));
+    index = menu->insertItem(i18n("Raise to &Top"), this, SLOT(raiseToTop()));
     rc = true;
+    if (_parent && _parent->_children.count() == 1) {
+      menu->setItemEnabled(index, false);
+    }
   }

   if (_layoutActions & LowerToBottom) {
-    menu->insertItem(i18n("Lower to &Bottom"), this, SLOT(lowerToBottom()));
+    index = menu->insertItem(i18n("Lower to &Bottom"), this, SLOT(lowerToBottom()));
     rc = true;
+    if (_parent && _parent->_children.count() == 1) {
+      menu->setItemEnabled(index, false);
+    }
   }

   if (_layoutActions & Rename) {

Comment 5 George Staikos 2006-01-27 23:23:13 UTC

On Friday 27 January 2006 16:50, Andrew Walker wrote:
> +    if (_parent && _parent->_children.count() == 1) {
> +      menu->setItemEnabled(index, false);
> +    }

   I think this part is wrong.  It should set the item disabled if it is 
already the lowest/highest, which means first or last in _parent->_children.

Comment 6 Andrew Walker 2006-01-27 23:38:04 UTC

Proposed patch addressing George's concerns:

Index: kstviewobject.cpp
===================================================================
--- kstviewobject.cpp   (revision 503013)
+++ kstviewobject.cpp   (working copy)
@@ -1080,7 +1080,8 @@

   bool rc = false;
   int id;
-
+  int index;
+
   _moveToMap.clear();

   if (!tagName().isEmpty()) {
@@ -1098,23 +1099,35 @@
   }

   if (_layoutActions & Raise) {
-    menu->insertItem(i18n("&Raise"), this, SLOT(raise()));
+    index = menu->insertItem(i18n("&Raise"), this, SLOT(raise()));
     rc = true;
+    if (_parent && !_parent->_children.empty() && _parent->_children.last().data() == this) {
+      menu->setItemEnabled(index, false);
+    }
   }

   if (_layoutActions & Lower) {
-    menu->insertItem(i18n("&Lower"), this, SLOT(lower()));
+    index = menu->insertItem(i18n("&Lower"), this, SLOT(lower()));
     rc = true;
+    if (_parent && !_parent->_children.empty() && _parent->_children.first().data() == this) {
+      menu->setItemEnabled(index, false);
+    }
   }

   if (_layoutActions & RaiseToTop) {
-    menu->insertItem(i18n("Raise to &Top"), this, SLOT(raiseToTop()));
+    index = menu->insertItem(i18n("Raise to &Top"), this, SLOT(raiseToTop()));
     rc = true;
+    if (_parent && !_parent->_children.empty() && _parent->_children.last().data() == this) {
+      menu->setItemEnabled(index, false);
+    }
   }

   if (_layoutActions & LowerToBottom) {
-    menu->insertItem(i18n("Lower to &Bottom"), this, SLOT(lowerToBottom()));
+    index = menu->insertItem(i18n("Lower to &Bottom"), this, SLOT(lowerToBottom()));
     rc = true;
+    if (_parent && !_parent->_children.empty() && _parent->_children.first().data() == this) {
+      menu->setItemEnabled(index, false);
+    }
   }

   if (_layoutActions & Rename) {

Comment 7 George Staikos 2006-01-27 23:49:06 UTC

  Looks good now.

Comment 8 Andrew Walker 2006-01-27 23:52:30 UTC

SVN commit 503033 by arwalker:

BUG:120882 Disable raise and lower menu items if they will have no effect.

 M  +18 -5     kstviewobject.cpp  


--- trunk/extragear/graphics/kst/kst/kstviewobject.cpp #503032:503033
@@ -1080,7 +1080,8 @@
 
   bool rc = false;
   int id;
-
+  int index;
+  
   _moveToMap.clear();
 
   if (!tagName().isEmpty()) {
@@ -1098,23 +1099,35 @@
   }
 
   if (_layoutActions & Raise) {
-    menu->insertItem(i18n("&Raise"), this, SLOT(raise()));
+    index = menu->insertItem(i18n("&Raise"), this, SLOT(raise()));
     rc = true;
+    if (_parent && !_parent->_children.empty() && _parent->_children.last().data() == this) {
+      menu->setItemEnabled(index, false);
+    }
   }
 
   if (_layoutActions & Lower) {
-    menu->insertItem(i18n("&Lower"), this, SLOT(lower()));
+    index = menu->insertItem(i18n("&Lower"), this, SLOT(lower()));
     rc = true;
+    if (_parent && !_parent->_children.empty() && _parent->_children.first().data() == this) {
+      menu->setItemEnabled(index, false);
+    }
   }
 
   if (_layoutActions & RaiseToTop) {
-    menu->insertItem(i18n("Raise to &Top"), this, SLOT(raiseToTop()));
+    index = menu->insertItem(i18n("Raise to &Top"), this, SLOT(raiseToTop()));
     rc = true;
+    if (_parent && !_parent->_children.empty() && _parent->_children.last().data() == this) {
+      menu->setItemEnabled(index, false);
+    }
   }
 
   if (_layoutActions & LowerToBottom) {
-    menu->insertItem(i18n("Lower to &Bottom"), this, SLOT(lowerToBottom()));
+    index = menu->insertItem(i18n("Lower to &Bottom"), this, SLOT(lowerToBottom()));
     rc = true;
+    if (_parent && !_parent->_children.empty() && _parent->_children.first().data() == this) {
+      menu->setItemEnabled(index, false);
+    }
   }
 
   if (_layoutActions & Rename) {