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
Alternatively the menu items should be disabled if they are not functional
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.
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.
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) {
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.
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) {
Looks good now.
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) {