View Issue Details

IDProjectCategoryView StatusLast Update
0017264ScribusUser Interfacepublic2024-09-16 20:54
Reporternitramr Assigned Tonitramr  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
PlatformDesktop PCOSUbuntuOS Version24.04 64-bit
Product Version1.6.2 
Target Version1.6.3.svnFixed in Version1.6.3.svn 
Summary0017264: ScrSpinBox: Alt key modifier doesn't work on Linux
DescriptionCurrently, the ScrSpinBox has different key modifier to influence the steps during mouse wheel scroll.

Scroll = 1
Scroll + Shift = 0.1
Scroll + Alt = 10.0
Scroll + Alt + Shift = 0.01

I found 3 issues with the current version of the ScrSpinBox

1. Alt Key
It looks like the Alt key modifier doesn't work for Linux (I guess Windows too). Usually if you press the Alt key in combination with another key you can select other controls in the UI (this is not the case for MacOS)

2. Ctrl Key
A regular QSpinBox already has an implementation. If you press Ctrl and scroll, it increases the steps by 10, similar to our Alt + scroll implementation)

3. Key modifiers during a scroll event permanently override the singleSteps() property. The up/down keys get the same single step, even if you set it differently for the widget.
In my opinion, the key modifier should only change the single step during a scroll event.

The attached patch includes the following changes:
Scroll = 1
Scroll + Shift = 0.1
Scroll + Ctrl = 10.0
Scroll + Ctrl + Shift = 0.01

Default Ctrl modifier has been disabled for mouse wheel scrolls.
Key modifier changes single step only temporary on scroll event
TagsNo tags attached.
PatchYes

Activities

nitramr

2024-09-08 18:02

developer  

scrspinbox_2024-09-08_01.patch (2,422 bytes)   
Index: scribus/ui/scrspinbox.cpp
===================================================================
--- scribus/ui/scrspinbox.cpp	(Revision 26266)
+++ scribus/ui/scrspinbox.cpp	(Arbeitskopie)
@@ -227,6 +227,7 @@
 	ts.replace("%", "");
 	ts.replace("°", "");
 	ts.replace(FinishTag, "");
+	ts.replace(suffix(), ""); // We have to remove custom suffix to get a valid input
 	ts = ts.trimmed();
 
 	if (ts.endsWith(su))
@@ -341,29 +342,36 @@
 		if (isReadOnly() || !hasFocus())
 			return true;
 		auto* wheelEvent = dynamic_cast<QWheelEvent*>(event);
+		int oldStep = singleStep(); // remember old single steps
 		bool shiftB = wheelEvent->modifiers() & Qt::ShiftModifier;
-		bool altB = wheelEvent->modifiers() & Qt::AltModifier;
+		bool altB = wheelEvent->modifiers() & Qt::ControlModifier;
+
 		if (shiftB && !altB)
-		{
 			setSingleStep((m_unitIndex == SC_INCHES) ? 0.0625 : 0.1);
-			retval = QAbstractSpinBox::event(event);
-		}
 		else if (!shiftB && altB)
-		{
 			setSingleStep((m_unitIndex == SC_INCHES) ? 1.0 : 10.0);
-			retval = QAbstractSpinBox::event(event);
-		}
 		else if (shiftB && altB)
-		{
 			setSingleStep((m_unitIndex == SC_INCHES) ? 0.03125 : 0.01);
-			retval = QAbstractSpinBox::event(event);
-		}
 		else
-		{
 			setSingleStep((m_unitIndex == SC_INCHES) ? 0.125 : 1.0);
-			retval = QAbstractSpinBox::event(event);
-		}
-		return retval;
+
+		// Reimplement MouseWheel behavior without CTRL modifier behavior of QAbstractSpinBox
+#ifdef Q_OS_MACOS
+		if ((wheelEvent->modifiers() & Qt::ShiftModifier) && wheelEvent->source() == Qt::MouseEventNotSynthesized)
+			wheelDeltaRemainder += wheelEvent->angleDelta().x();
+		else
+			wheelDeltaRemainder += wheelEvent->angleDelta().y();
+#else
+		wheelDeltaRemainder += wheelEvent->angleDelta().y();
+#endif
+		const int steps = wheelDeltaRemainder / 120;
+		wheelDeltaRemainder -= steps * 120;
+		if (stepEnabled() & (steps > 0 ? StepUpEnabled : StepDownEnabled))
+			stepBy(steps);
+
+		setSingleStep(oldStep);
+
+		return true;
 	}
 
 	if (event->type() == QEvent::LocaleChange)
Index: scribus/ui/scrspinbox.h
===================================================================
--- scribus/ui/scrspinbox.h	(Revision 26266)
+++ scribus/ui/scrspinbox.h	(Arbeitskopie)
@@ -54,6 +54,7 @@
 	
 	protected:
 		uint m_unitIndex { 0 };
+		int wheelDeltaRemainder;
 		const QMap<QString, double>* m_constants { nullptr };
 
 		void setParameters(int s);

scrspinbox_2024-09-08_01.patch (2,422 bytes)   

ale

2024-09-09 14:31

manager   ~0051315

yes, if it's indeed trying to use alt, it's a good idea to replace it by ctrl-shift (this is what is used when moving a frame by 0.01 on the canvas)

but I also would rename the variable from altB to ctrlB to avoid future people to wonder why alt does not work there before noticing that alt is a ctrl...

nitramr

2024-09-10 16:52

developer   ~0051324

variable changed from altB to ctrlB.
scrspinbox_2024-09-10_01.patch (3,006 bytes)   
Index: scribus/ui/scrspinbox.cpp
===================================================================
--- scribus/ui/scrspinbox.cpp	(Revision 26266)
+++ scribus/ui/scrspinbox.cpp	(Arbeitskopie)
@@ -227,6 +227,7 @@
 	ts.replace("%", "");
 	ts.replace("°", "");
 	ts.replace(FinishTag, "");
+	ts.replace(suffix(), ""); // We have to remove custom suffix to get a valid input
 	ts = ts.trimmed();
 
 	if (ts.endsWith(su))
@@ -330,8 +331,7 @@
 
 bool ScrSpinBox::eventFilter(QObject* watched, QEvent* event)
 {
-	bool retval = false;
-/* Adding this to be sure that the IM* events are processed correctly i.e not intercepted by our KeyPress/Release handlers */
+	/* Adding this to be sure that the IM* events are processed correctly i.e not intercepted by our KeyPress/Release handlers */
  	if (event->type() == QEvent::InputMethod)
  		return QDoubleSpinBox::eventFilter(watched, event);
 
@@ -341,29 +341,36 @@
 		if (isReadOnly() || !hasFocus())
 			return true;
 		auto* wheelEvent = dynamic_cast<QWheelEvent*>(event);
+		int oldStep = singleStep(); // remember old single steps
 		bool shiftB = wheelEvent->modifiers() & Qt::ShiftModifier;
-		bool altB = wheelEvent->modifiers() & Qt::AltModifier;
-		if (shiftB && !altB)
-		{
+		bool ctrlB = wheelEvent->modifiers() & Qt::ControlModifier; // for macOS it is CMD button
+
+		if (shiftB && !ctrlB)
 			setSingleStep((m_unitIndex == SC_INCHES) ? 0.0625 : 0.1);
-			retval = QAbstractSpinBox::event(event);
-		}
-		else if (!shiftB && altB)
-		{
+		else if (!shiftB && ctrlB)
 			setSingleStep((m_unitIndex == SC_INCHES) ? 1.0 : 10.0);
-			retval = QAbstractSpinBox::event(event);
-		}
-		else if (shiftB && altB)
-		{
+		else if (shiftB && ctrlB)
 			setSingleStep((m_unitIndex == SC_INCHES) ? 0.03125 : 0.01);
-			retval = QAbstractSpinBox::event(event);
-		}
 		else
-		{
 			setSingleStep((m_unitIndex == SC_INCHES) ? 0.125 : 1.0);
-			retval = QAbstractSpinBox::event(event);
-		}
-		return retval;
+
+		// Reimplement MouseWheel behavior without CTRL modifier behavior of QAbstractSpinBox
+#ifdef Q_OS_MACOS
+		if ((wheelEvent->modifiers() & Qt::ShiftModifier) && wheelEvent->source() == Qt::MouseEventNotSynthesized)
+			wheelDeltaRemainder += wheelEvent->angleDelta().x();
+		else
+			wheelDeltaRemainder += wheelEvent->angleDelta().y();
+#else
+		wheelDeltaRemainder += wheelEvent->angleDelta().y();
+#endif
+		const int steps = wheelDeltaRemainder / 120;
+		wheelDeltaRemainder -= steps * 120;
+		if (stepEnabled() & (steps > 0 ? StepUpEnabled : StepDownEnabled))
+			stepBy(steps);
+
+		setSingleStep(oldStep);
+
+		return true;
 	}
 
 	if (event->type() == QEvent::LocaleChange)
Index: scribus/ui/scrspinbox.h
===================================================================
--- scribus/ui/scrspinbox.h	(Revision 26266)
+++ scribus/ui/scrspinbox.h	(Arbeitskopie)
@@ -54,6 +54,7 @@
 	
 	protected:
 		uint m_unitIndex { 0 };
+		int wheelDeltaRemainder;
 		const QMap<QString, double>* m_constants { nullptr };
 
 		void setParameters(int s);

scrspinbox_2024-09-10_01.patch (3,006 bytes)   

Issue History

Date Modified Username Field Change
2024-09-08 18:02 nitramr New Issue
2024-09-08 18:02 nitramr File Added: scrspinbox_2024-09-08_01.patch
2024-09-08 19:31 nitramr Description Updated
2024-09-09 14:31 ale Note Added: 0051315
2024-09-10 16:52 nitramr Note Added: 0051324
2024-09-10 16:52 nitramr File Added: scrspinbox_2024-09-10_01.patch
2024-09-12 19:30 cbradney Assigned To => nitramr
2024-09-12 19:30 cbradney Status new => resolved
2024-09-12 19:30 cbradney Resolution open => fixed
2024-09-12 19:30 cbradney Fixed in Version => 1.6.3.svn
2024-09-16 20:54 cbradney Status resolved => closed