Skip to content

Conversation

corranwebster
Copy link
Contributor

Scrolling was in the reverse direction for vertical scrollbars (ie. when scrollbar was at top, view was at bottom and vice-versa). This fixes this issue.

Testing is difficult for this - have just spent an unproductive hour trying to get something working. We probably want a little set of utility routines. I'll leave it up to the PR reviewer as to whether we should try to add tests before merging.

@jwiggins
Copy link
Member

So the value variables here are coming from a GraphicsContext coordinate system? If so, utility routines are a very good idea. This conversion comes up all the time in enable.

@itziakos
Copy link
Member

I think that using the https://github.com/enthought/pyface/blob/master/pyface/ui/qt4/util/modal_dialog_tester.py on should be able to verify that the scrobars have the right value.

@corranwebster
Copy link
Contributor Author

@jwiggins In the case I'm looking at, the value is coming from a GraphicsContext, but it doesn't have to - it can be any arbitrary quantity being controlled by the scrollbar. Enable expects the position of a vertical scrollbar to go bottom to top, but Qt expects top down, but the values are arbitrary.

Nevertheless, I've pulled the computation out into something re-usable and hopefully clearer.

@corranwebster
Copy link
Contributor Author

Hrm... this probably isn't right if low isn't 0.

@corranwebster corranwebster mentioned this pull request Aug 21, 2015
@corranwebster
Copy link
Contributor Author

Added a test and corrected for non-zero min values.

I think that this is ready to merge.

@jwiggins
Copy link
Member

Thanks Corran! I'll take a look later during the inevitable Travis pauses.

@corranwebster
Copy link
Contributor Author

Hrmmm... need to skip the new test when running under wx.

Copy link
Member

Choose a reason for hiding this comment

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

enable_range and value are being ignored here.

@jwiggins
Copy link
Member

I think it might still fail when it actually runs the tests. You should look at the previous run with Qt.

@corranwebster
Copy link
Contributor Author

Hmmm... yes, looks like the window isn't being created, but the tests are running fine on my machine. Not sure what's going on.

@corranwebster
Copy link
Contributor Author

A bit more digging, it looks like _size isn't being set because the _paint method doesn't get called in Travis. Trying to use the event loop to get the window set up properly.

@jwiggins
Copy link
Member

Sounds good. Once Travis is satisfied, I'm ready to merge this.

@corranwebster
Copy link
Contributor Author

With fixes for unexpected success from master, the tests are now passing, so I think this is good.

@jwiggins
Copy link
Member

LGTM!

jwiggins added a commit that referenced this pull request Sep 12, 2015
Fix scrolling problem with Qt native scrollbar
@jwiggins jwiggins merged commit 85f9e90 into master Sep 12, 2015
@jwiggins jwiggins deleted the fix/qt-native-scrollbar branch September 12, 2015 08:52
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.

3 participants