haveno-dex
scrollbar fix, but better
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.
Thanks. Looking at the screenshot, the Pay by Mail text areas are also too large, compared to current master:
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.
It's looking much better. 🎉
Is it possible to remove this extra padding below the confirmation buttons?
That should negate the need for the scrollbar for most payment methods. Ideally there would be 0 extra padding under the buttons.
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.
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
The last commit should be functioning as intended. Here are the things that (should) be working now: -Vertical scrolling on the entire subview, instead of just on the form area (this fixes cases which would hide the matrix channel text below the support ticket button) -Scrolling (finally) applies to the entire view with no (okay there is one, see below) janky workarounds -The numerous layout issues I was having with TextAreas (not scaling properly, ignoring restraints, vertically centering even when explicitly told not to, etc etc) should be fixed by adding an invisible TextArea to the beginning of the subview. As far as I can tell, the TextArea doesn't mess with the layout of the form or block interaction with elements. This is necessary as the TextArea issues only occur to the very first instance of a TextArea within the view. -(Extra one I forgot about originally): This also currently sets the maximum height of the table to 100; This can be increased if needed, but dynamically sizing the table will almost always result in scrolling being required on the lower view, regardless of screen size.
I think this is ready to be looked at. If this works properly, I can squash the commits.
I'm seeing a strip of blank padding added to the bottom of the screen, as shown by this cut off text and divider lines:
Versus current master which stretches to the end of the screen:
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).
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.
I am unable to reproduce this behavior; The following screenshots are from me building and running clean using java 21.0.2.fx-librca on MacOS 14.2.1:
This is with the initial window dimensions
This is with the minimum window dimensions
Oops, I must have rebuilt incorrectly. It's showing like your sceenshots now.
However, we want to preserve the same behavior that's currently on master and Bisq, where ~2 trades are shown by default, and the list height grows a little if the user increases the height of the application.
Otherwise, all this screen real estate is unused:
Hopefully it's possible without too much trouble.
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:
tableView.setPrefHeight(100);
tableView.setMaxHeight(200);
So let's add that change and I think it's ready to merge. :)