-
-
Notifications
You must be signed in to change notification settings - Fork 130
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
scrollbar fix, but better #773
Conversation
I believe that this should (finally) fix the issue. It seems that the issue all along was that the submit payment button wasn't having its height set properly (kinda), and so the HBox enclosing it would also not have the right height set, which would then cause the whole thing to render as if it had no height, despite it very clearly having height. Adding ~90 padding plus top-left aligning the button in the HBox fixes the issue. |
I tried to do that, but the scrollpane refuses to grow unless the hBox is given extra height. This could be done either using padding or by setting the minimum height of the hBox, which both give the same effect. The padding/height can be reduced further, but below 50 the scrollpane will begin cutting off the button again. I used the padding instead of the height, as it makes it more clear that it is intended to be padding. I suspect that the issue is somehow related to the button implementation in JFoenix, although I am unsure. |
Unfortunately the default view for most payment methods would have this unnecessary scrollbar with extra height at the bottom, whereas they didn't before, so it would be a step backwards in terms of overall user experience. |
I’ll look and see what I can do for that |
Thanks for your help. |
Any luck @preland? Hoping this can make it in before release in the next few weeks. |
I'm still working on it. I actually had it working almost perfectly (except there was a weird gap between the peer and main addresses) but it seems to have undone itself. I'm not going to make any conjecture as to what's causing it, because I no longer think it's a single issue. I think I'll be able to get it done by this Friday |
Happy to re-test if it's ready. Please let me know. |
The last commit should be functioning as intended. Here are the things that (should) be working now: I think this is ready to be looked at. If this works properly, I can squash the commits. |
This should fix that issue |
It seems to be very close. The scrollbar appears correctly for a taller trade. Only nit: when I click back to the first short offer again, the scrollbar remains vislbe, even though it didn't appear the first time and isn't necessary. |
Interesting; I'm not able to reproduce that behavior myself, and looking into the code further, it appears that each element gets its own scrollview so this shouldn't be happening anyways. Were the dimensions of the window default, or something else? Also, for future reference, what Java version are you using (I use 11.0.21-tem from sdkman). |
Can you provide an update on the status of this issue? |
The scrollbar is correctly appearing and disappearing after updating my JDK. Thanks. My only request, if possible, is to restore previous behavior where the height of the trades list grows a little when the user grows the height of the application. Otherwise with this PR, the trades list is fixed to always show only 2 trades, even if the user is in full screen mode and may have many trades. |
The limit has now been removed |
@preland Any updates? |
This commit should fix the aforementioned issue |
Great, it's working. Let's cap the max height to prevent unbound growoth of the trades list. I tested by setting the max height after the preferred height and it works:
So let's add that change and I think it's ready to merge. :) |
The change has been made; should I squash commits? |
Yes please. |
Commits are now squashed |
This PR is a reimplementation of the scrollbar fix for the pending trades screen. It is a lot simpler than the earlier solution, while also maintaining higher functionality. The width issue should be addressed now, such that a horizontal scrollbar should no longer be necessary (at least for Pay By Mail; if another method requires horizontal scrolling it could be added back in). The width and height of the text sections now properly scale on different screen sizes.
The only issue I can find is a minor layout issue on Pay By Mail in certain conditions:
In some conditions, it will look fine:

In other conditions, it will look like this:

My guess is that is has something to do with the placeholder text being one column, while the form is two columns. The issue occurs consistently when moving from a placeholder view to the form view, while moving from a two column view (in my testing i used the Zelle form) to the form resulted in no changing. The issue only consistently reverts when moving from another tab back to the form. I was also able to get the issue to fix itself ~5% of the time by using an autoclicker and switching between tabs at around 20 cps, which I am unable to explain currently.