Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Increase "Proposals of current cycle" table size to fit the window #3318

Closed
wants to merge 1 commit into from
Closed

Increase "Proposals of current cycle" table size to fit the window #3318

wants to merge 1 commit into from

Conversation

niyid
Copy link
Contributor

@niyid niyid commented Sep 24, 2019 •
edited by freimair
Loading

fixes #3229

freimair
freimair suggested changes Nov 1, 2019
View reviewed changes
Copy link
Member

@freimair freimair left a comment

Choose a reason for hiding this comment

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

thanks for your contribution, however, I just fired your branch up and I am seeing no difference to before.

Screenshot from 2019-11-01 09-10-08

maybe this can be done similarly to the fixed done in #3077

@niyid
Copy link
Contributor Author

niyid commented Nov 1, 2019 via email

@niyid
Copy link
Contributor Author

niyid commented Nov 1, 2019

The list-view previously left a lot of space outside of the scroll panel. Now the space is well utilized and will extend further with more items in the list-view. The difference is as shown below:

Screenshot from 2019-11-01 14-35-22

@niyid
Copy link
Contributor Author

niyid commented Nov 7, 2019

@freimair

Please check the screenshot I got on applying the PR below:

Screenshot from 2019-11-07 23-56-04

Compare to the screenshot without applying the PR below:

Screenshot from 2019-11-08 00-00-35

@niyid niyid requested a review from freimair November 7, 2019 23:06
@freimair
Copy link
Member

freimair commented Nov 9, 2019

just checked again, nothing changed on my side. please be aware that I am on Linux Gnome Shell here. Maybe @devinbileck (windows) and @ripcurlx (macos) can have a look too?

@niyid
Copy link
Contributor Author

niyid commented Nov 11, 2019 •
edited
Loading

@freimair

Hi. Can we clear this up on Slack? I have sent you a few messages.

@ripcurlx @devinbileck
I will appreciate if further tests can be conducted on other systems. BTW I am on Fedora 22.

ripcurlx
ripcurlx suggested changes Nov 12, 2019
View reviewed changes
Copy link
Contributor

@ripcurlx ripcurlx left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@niyid Your fix improves on first sight the height of the open proposals, but as soon as the blind voting is on it hides an important part behind off screen.
Bildschirmfoto 2019-11-12 um 13 19 58
Bildschirmfoto 2019-11-12 um 13 20 03

Master in Blind vote phase:
Bildschirmfoto 2019-11-12 um 13 31 22

The right way to fix this would be to adapt the double initialProposalTableViewHeight = 180; and tableView.setPrefHeight(100); based on the proposal phase, as atm it resizes only correctly in the Blind vote phase.

@niyid
Copy link
Contributor Author

niyid commented Nov 12, 2019 via email

@niyid
Copy link
Contributor Author

niyid commented Nov 12, 2019

Thanks for the feedback. I will adopt your suggestion. Regards.
…

@ripcurlx

Since this fix only appears to work in the BLIND_VOTE phase, I could make the height as is for the rest and resize to double in the BLIND_VOTE phase. Does that work?

        double initialProposalTableViewHeight = 180;
    	if(DaoPhase.Phase.BLIND_VOTE.equals(currentPhase)) {
    		initialProposalTableViewHeight = 360;
    	}        

        tableView.setPrefHeight(100);
    	if(DaoPhase.Phase.BLIND_VOTE.equals(currentPhase)) {
            tableView.setPrefHeight(200);
    	}

@ripcurlx
Copy link
Contributor

ripcurlx commented Dec 6, 2019

Since this fix only appears to work in the BLIND_VOTE phase, I could make the height as is for the rest and resize to double in the BLIND_VOTE phase. Does that work?

Did it work for you when you tried it out?

@niyid
Copy link
Contributor Author

niyid commented Dec 6, 2019 via email

@niyid
Copy link
Contributor Author

niyid commented Dec 6, 2019

Since this fix only appears to work in the BLIND_VOTE phase, I could make the height as is for the rest and resize to double in the BLIND_VOTE phase. Does that work?

Did it work for you when you tried it out?

Can you please point me to any resources on testing the GUI on the TESTNET?

I found this: bisq-network/proposals#74 but I need specifics on how to pass data to target views - mock test.

@julianknutsen
Copy link
Contributor

@niyid Check out https://github.com/bisq-network/bisq/blob/master/Makefile that was recently added to master to help the onboarding of localnet Bisq clusters that are useful for testing/debugging.

We don't currently have a testing framework for unit testing, but it isn't too complicated to bring up a localnet and move it through the proposal phases to verify the behavior.

@niyid
Copy link
Contributor Author

niyid commented Dec 6, 2019 via email

@ripcurlx ripcurlx mentioned this pull request Dec 13, 2019
Closed
@stale
Copy link

stale bot commented Jan 5, 2020

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the was:dropped label Jan 5, 2020
@stale
Copy link

stale bot commented Jan 12, 2020

This issue has been automatically closed because of inactivity. Feel free to reopen it if you think it is still relevant.

@stale stale bot closed this Jan 12, 2020
@ripcurlx ripcurlx mentioned this pull request Jan 13, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ripcurlx ripcurlx ripcurlx requested changes

@sqrrm sqrrm Awaiting requested review from sqrrm

@freimair freimair Awaiting requested review from freimair

Assignees
No one assigned
Labels
waiting for author was:dropped
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[UI] Increase "Proposals of current cycle" table size to fit the window
4 participants
@niyid @freimair @ripcurlx @julianknutsen