-
-
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
Fix scrollbar in Open Trades screen [1 XMR] #677
Comments
There is a bounty on this issue, the amount is in the title. The reward will be awarded to the first person or group of people who resolves this issue. If you are starting to work on this bounty, please write a comment, so that we can assign the issue to you. We expect contributors to provide a PR in a reasonable time frame or, in case of an extensive work, updates on their progresses. We will unassign the issue if we feel the assignee is not responsive or has abandoned the task. Read the full conditions and details of our bounty system. |
Is this issue still open, because I see that #673 has been mered |
Yes this issue still applies. |
Hello, |
Ok, I've assigned it to you. |
I don't have the time needed to do this anymore. |
I can replicate the behavior described with that fix I think it has something to do with the footer being given a minimum height at runtime, but not at initialization. Because of this, it will scroll at first, but will increase the size of the footer upon reload. I was only able to figure this out because when I first compiled the fix, I had manually set the size of the window to be a little short, and the scrollbar showed up at first, but going back to it instantly forced the size of the window to become larger and removed the scrollbar. It rarely, if ever, showed up again on further launches Very weird behavior indeed Just so I am clear on what is wanted: Do you want a scrollbar for the entire window (what SweetBearKiller did) or a scrollbar just for the trade information section? |
I think ideally the scrollbar would be only for the trade information section, since the trades already have a scrollbar, and so that would create a nested scrollbar with the entire window. But something is better than nothing, since this is a usability issue which is still present. Very happy to have it looked at. Thanks. |
It appears this is a byproduct of the app not scaling very well with differing resolutions; this seems to be an issue with all of the pages, though the open trades menu is the most egregious example Here are some resolution examples: The UI appears to look best in ~1080p resolution with a ~16:9 aspect ratio. Even at 1440p, there begins to be scaling issues. It appears that the trade info section was already a scrollarea, but the scrollbar had been set to never appear. Changing this causes it to appear, but it will simply scale with the trade-info area, even if extra info is off-screen. |
An update on what I’ve been doing: I’m currently working on changing the UI to adapt better to differing resolutions and aspect ratios, with the end goal to get the lowest resolution image above to present (by default) similar to the highest resolution image. You can check on my progress on the master branch of my fork if you are ever curious (I’ll commit whatever I’ve gotten finished every day or so regardless of the current state of the project; currently the ui is incredibly buggy because the dynamic scaling hasn’t been implemented yet for the main “building blocks” of the ui) TL;DR I probably could’ve fixed this issue by manually changing the display type on this specific screen to hyper-specifically fix this issue, but the main issue at hand would keep showing up as the app enters production. |
If that proves to be a rabbit hole, some hacky fix to this specific problem would be fine / best for basic functionality before release. |
I’ll try and do that then. I’ll keep my current work on a separate branch, as it could allow for a lot of QOL things that aren’t currently possible (such as changing the text size) |
That PR should fix the issue |
Bounty bumped to 0.50 XMR. |
As I've been working on it, I've been having issues with repeatedly replicating the issue, especially when it comes to the width/height of the text boxes (In the case of the Pay By Mail form, that would be the address and additional info). My guess is that it is somewhat related to that, as well as something else weird going on when the submit payment button is combined with the form to create the full screen. To make sure I'm not unnecessarily wasting time during development, is there any sort of "debug" build option that populates the tables with test data? In addition, is there a way to quickly view UI changes without fully recompiling the entire jar (for context, I have been using a local test network with manually inputted data, and have been recompiling with "make skip-tests" and executing with "make user{1 or 2}-desktop-local" whenever I make a change). |
Sure, what I do is run a test from the haveno-ts library which sets up the arbitrator and traders, funds their wallets, and initiates a bunch of trades with different currencies and payment methods.
After this the desktop apps can be opened to verify default state.
There is also |
Bounty bumped to 1 XMR. |
@preland I've assigned the issue to you. If you decide to not complete the issue, please let me know and I can un-assign for someone else. |
Sounds good |
Fixed with #773 |
This issue requests fixing the scrollbar to the Open Trades screen, which can cut off the button to send payment, especially with long payment methods like Pay by Mail or on low resolution screens.
For example:
The text was updated successfully, but these errors were encountered: