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.

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

Add masternode tab in qt wallet #776

Merged
merged 6 commits into from
May 19, 2016
Merged

Add masternode tab in qt wallet #776

merged 6 commits into from
May 19, 2016

Conversation

UdjinM6
Copy link

@UdjinM6 UdjinM6 commented May 11, 2016

What's inside:

  • show all masternodes
  • filter masternode list (updates every MASTERNODELIST_UPDATE_SECONDS (5) seconds)
  • show "my" masternodes (from masternode.conf)
  • start alias/all/MISSING for "my" masternodes
  • status of "my" mastenordes updates every MY_MASTERNODELIST_UPDATE_SECONDS (60) seconds (could be cpu-heavy for large lists)

Tab will be active only if there is at least one legit line in masternode.conf, normal users should see no difference/performance impact.

screen shot 2016-05-11 at 12 57 36

screen shot 2016-05-11 at 14 19 22

nDurationTime /= 60;
int hours = nDurationTime % 24;
int days = nDurationTime / 24;
if((hours == 0)&&(days == 0))

Choose a reason for hiding this comment

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

Nit: Would be nice to have some extra whitespace around the &&.

Might also consider removing inner parenthesis as similar conditions in this project don't seem to use them (eg: addresstablemodel.cpp).

Copy link
Author

Choose a reason for hiding this comment

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

agree, rewritten this part, see 3764e68#diff-8e9b29a6133dd7bf43643c37c5c4689aR94

Choose a reason for hiding this comment

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

utACK Cheers!

@schinzelh
Copy link

concept ACK, i love it

@jonathancross
Copy link

Thanks @UdjinM6! Assuming the list of "All Masternodes" will be very long, should we make "My Masternodes" the default (and move to the left position)?

The user would need to take action (click) on "All Masternodes" in order to initiate display of the long list which seems like better UX.

@schinzelh
Copy link

I agree, "My Masternodes" should be default tab.

@UdjinM6
Copy link
Author

UdjinM6 commented May 11, 2016

Nit addressed 3764e68#diff-8e9b29a6133dd7bf43643c37c5c4689aR94, tabs swapped and I also removed few debug printfs (no idea how they sneaked here 😄 )

@crowning-
Copy link

Love it as well 👍

Good thing is, I wanted to implement something like this myself and fortunately didn't yet start. Time saved to break other things 😎

@UdjinM6
Copy link
Author

UdjinM6 commented May 13, 2016

Looking forward for a new icon for Masternodes tab from @snogcel 😄 (will also require slight code changes)

@crowning-
Copy link

Builds and runs as expected on both operating systems (and Windows also 😈 , tested with 8.1 and 10).

Protocol column needs some more room, and making the column width adjustable would be nice (but I can live without that):
clipboard01

- set initial columns size
- use resizable columns
- use theme-dependent masternodes icon (placeholder, should be replaced)
- removed unused event binding
- capitalization, more verbose ui element names
@UdjinM6
Copy link
Author

UdjinM6 commented May 15, 2016

@crowning- : agree, pushed new fixes in additional commit (will squash all commits into one and rebase if needed later).
Also fixed a small bug (event binding), done some trivial refactoring (more verbose ui element names) and added my ugly theme-dependent masternodes icon (will replace it when there will be a good alternative - @salmion, pls 😉 ).

…be set to true at first run for those who have masternodes in their masternode.conf and false for others.
@UdjinM6
Copy link
Author

UdjinM6 commented May 15, 2016

Was not quite happy with no way to control Masternodes tab appearance so added a semi-automatic gui option.

screen shot 2016-05-16 at 1 46 37

PS. someone, pls, double check my English for tooltip here and for a "Note" in "My Masternodes" sub-tab

@UdjinM6
Copy link
Author

UdjinM6 commented May 15, 2016

Icon updated, thanks @salmion!
screen shot 2016-05-16 at 2 05 03
screen shot 2016-05-16 at 2 05 10

@crowning-
Copy link

Shouldn't conceptually the "Show Masternodes Tab" checkbox be right below the "Enable coin control features" checkbox?

@UdjinM6
Copy link
Author

UdjinM6 commented May 16, 2016

@crowning- probably... wasn't sure about where to place it but yeah, I guess that will be a better place. moved :)

@UdjinM6
Copy link
Author

UdjinM6 commented May 16, 2016

found and fixed a dumb bug, see 86bca13

@crowning-
Copy link

crowning- commented May 16, 2016

@UdjinM6 if we'd ever meet in real life having a beer and I would tell you just HOW often I have connected wrongly attributed signals in Qt you'd have a LOT of beers when I'm finished...:stuck_out_tongue_winking_eye:

@eduffield222
Copy link

This is greeeat!!! 👍 👍 👍

@eduffield222 eduffield222 merged commit 1376208 into dashpay:v0.12.1.x May 19, 2016
@eduffield222
Copy link

Merged!

@schinzelh
Copy link

Bummer, i wanted this to be squashed first :) N/m

@UdjinM6 UdjinM6 deleted the masternodeTab branch June 20, 2016 06:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants