Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A different approach to avoiding accidently scroll changes #4274

Merged
merged 3 commits into from Mar 21, 2017

Conversation

nyalldawson
Copy link
Collaborator

@nyalldawson nyalldawson commented Mar 17, 2017

Here's a different approach to swallowing up scroll events to prevent accidental widget changes. This one sets a time out, so that any wheel events which occur up to 1 sec after a scroll wont change a child widget's value. This still allows the handy hover-and-scroll-wheel-to-change-value shortcut, just not directly after a scroll has occurred. Fixes the issue for me...

@@ -0,0 +1,110 @@
/***************************************************************************
qgspanelwidget.cpp
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better use file templates in QtCreator ;)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't let the general crappiness of the coding in this PR distract from the bigger question. I deleted the PR guideline checklist for a reason ;)

---------------------
begin : June 2016
copyright : (C) 2016 by Nathan Woodrow
email :
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@NathanW2 they will also find your email, even if you don't add it here!

@m-kuhn
Copy link
Member

m-kuhn commented Mar 17, 2017

I just experimented with this for another use.

All children that are registered are either "QObject" or "QWidget" but no futher subclasses. Maybe something related to only the superclass constructors being run but not the subclasses one?

@nyalldawson
Copy link
Collaborator Author

All children that are registered are either "QObject" or "QWidget" but no futher subclasses. Maybe something related to only the superclass constructors being run but not the subclasses one?

I think that's expected - according to the qt docs:

"In both cases you can only rely on the child being a QObject (or, if QObject::isWidgetType() returns true, a QWidget). This is because in the QEvent::ChildAdded case the child is not yet fully constructed; in the QEvent::ChildRemoved case it might have already been destructed."

But that would only be an issue if somewhere in the child constructors the event filter list is cleared. That doesn't seem likely to me.

I think I know what the issue is - I think that in some cases when a widget is being added to the scroll area, it already has children added. In this case the recursive filter install isn't added for all the existing children. Should be an easy fix if this is the case - I'm testing now.

@nyalldawson
Copy link
Collaborator Author

All fixed up now - a nice and clean approach which should be good merge.

@3nids
Copy link
Member

3nids commented Mar 20, 2017

great, can you make a custom widget for it?

This adds a new QgsScrollArea widget which is a subclass of QScrollArea.
QgsScrollArea has extra logic which temporarily blocks wheel events
from hitting child widgets for a short period following a scroll
of the area. This means that when scrolling a scroll area using the
mouse wheel, values won't be accidentally changed if the mouse
cursor momentarily lands on top of a widget.

QScrollArea should no longer be used in any QGIS code or plugins,
instead use QgsScrollArea to benefit from this fix.
@nyalldawson
Copy link
Collaborator Author

@3nids

great, can you make a custom widget for it?

Done!

@3nids
Copy link
Member

3nids commented Mar 21, 2017

thanks a lot
let's merge

@nyalldawson nyalldawson merged commit aad182f into qgis:master Mar 21, 2017
@nyalldawson nyalldawson deleted the scroll_timer branch July 16, 2017 23:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants