-
Notifications
You must be signed in to change notification settings - Fork 812
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
Support touch scroll in register; remove legacy scroll interactions #1822
base: stable
Are you sure you want to change the base?
Conversation
Just as an experiment, I'd like to compare how standard scrolling behavior feels in the register view. I find the current fast and quantized scroll to be disconcerting, but I recognize that a lot of thought, and trial and error, have already gone into the existing custom scroll interactions. This patch replaces custom scrolling with a ScrolledWindow container and mostly the default handlers.
9d1e648
to
27d2c5b
Compare
@adamwight What are you testing on ? The vertical scrollbar will not move up any further even though there a three more transactions above. Would need to decide what to do with the white space in top right, maybe useful for something? This time I have shrunk the width on Gnucash, again the vertical scrollbar is wrong and also the white space is gone. This shows the horizontal scroll bar over the last transaction with the vertical at the bottom. |
@adamwight We've been running this experiment for 20 years. All of the special-case code is there to deal with a problem some user had. Unless you have hardware to test with several input devices (trackpad, trackball, mouse, graphics tablet) on both Linux windowing systems (XOrg and Wayland) as well as Windows and macOS and the time to test all of those combinations you're not going to be able to detect regressions. Expecting the developers to do that for you is an unfair demand on their time. BTW, mac trackpads behave as "touch" devices for the purposes of "touch scroll". Works fine with the GnuCash register, I've been using them for 6-7 years. |
I was able to reproduce @Bob-IT 's regression by setting @jralls Tremendous work done on this package so far! I was intrigued by what I've seen, and it seems to be a friendly and helpful community. I'm familiar with some of the challenges of maintaining a legacy code base and I tend to agree with you that tearing everything to the ground is rarely a good idea. The reason I made this patch was because I noticed two current gnucash quirks that marred my otherwise pleasant user experience:
Looking into the code, I see quite a lot of special handling for various types of scrolling, but only for the register view. It suggests a smell, especially since this is the view which is giving me problems. As a step in debugging, I disconnected all of the custom scroll logic and this resulted in a surprisingly nice standard interaction, at least to my own taste. If project developers recommend that I rework the patch to strictly add touchscreen support without affecting the existing customizations, I'd be happy to do so. Or if touchscreen isn't an imagined use case then it's fine to drop the investigation, too. |
Oh, it's smelly alright. That's because it's far too customized for its own good. @Bob-IT tried to rewrite it based on standard GtkWidgets a few years ago but getting all of the behaviors close enough to the original to keep users happy turned out to be too hard and he set it aside to bitrot.
See the comment and code starting at
What platform is that and what's the problem?
Touchscreen use hasn't come up before, so let's try to figure out what's going wrong before we decide whether fixing it is too hard. Making sure that there aren't any regressions for anybody else definitely falls into the too hard category though so please do focus on getting your touchscreen working. |
Just as an experiment, I'd like to compare how standard scrolling behavior feels in the register view. I find the current fast and quantized scroll to be disconcerting, but I recognize that a lot of thought, and trial and error, have already gone into the existing custom scroll interactions.
This patch replaces custom scrolling with a ScrolledWindow container and its default handlers.
Reviewers: if you like this simplification, please suggest any legacy scrolling behaviors which should be reinstated.