Skip to content
This repository has been archived by the owner on Mar 23, 2023. It is now read-only.

refactor: hide delegate tab if wallet is not on profile #1438

Merged
merged 16 commits into from
Oct 15, 2019
Merged

refactor: hide delegate tab if wallet is not on profile #1438

merged 16 commits into from
Oct 15, 2019

Conversation

dated
Copy link
Contributor

@dated dated commented Sep 4, 2019

  1. explicitly set cursor-pointer on anchors in the wallet address component (the pointer is missing on the transaction show modal for instance)
  2. hide the delegate tab if the current wallet is not owned

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Refactor
  • Performance
  • Tests
  • Build
  • Documentation
  • Code style update
  • Continuous Integration
  • Other, please describe:

Does this PR introduce a breaking change?

  • Yes
  • No

Does this PR release a new version?

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

  • It's submitted to the develop branch, not the master branch
  • All tests are passing
  • New/updated tests are included

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

@ghost
Copy link

ghost commented Sep 4, 2019

Thanks for submitting this pull request! A maintainer will review this in the next few days and explicitly select labels so you know what's going on.

If no reviewer appears after a week, a reminder will be sent out.

@ghost ghost added Complexity: Low Less than 64 lines changed. Type: Refactor The pull request improves or enhances an existing implementation. labels Sep 4, 2019
Copy link
Contributor

@luciorubeens luciorubeens left a comment

Choose a reason for hiding this comment

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

It's useful for viewing the vote, this info should appear at least elsewhere before we hide the tab.

@alexbarnsley
Copy link
Member

Perhaps make use of the vote status bar, hide the vote/unvote & search buttons

@alexbarnsley
Copy link
Member

Actually, that only shows on the delegate tab so I'm not sure where you would show it 🤔

@dated
Copy link
Contributor Author

dated commented Sep 10, 2019

Actually the status bar doesn't show for unowned wallets, so that information isn't necessarily obtainable from the first page of the delegate table either.

One could repurpose the dot that shows up in the wallet header when a wallet is not voting to show that a wallet is in fact voting, while displaying the delegate name on hover. But that might also require some modification to the red dot in the wallet table, which currently holds a conflicting type of information (as it shows up only if a voted delegate is out of the top X).

@alexbarnsley alexbarnsley added the Status: Needs Changes The pull request needs additional changes before it can be merged. label Sep 16, 2019
@ghost
Copy link

ghost commented Sep 16, 2019

Your pull request needs some changes. Please wait for a comment from one of our developers for more information.

@dated
Copy link
Contributor Author

dated commented Sep 22, 2019

Opinions anyone? :D

@alexbarnsley
Copy link
Member

🤔 not sure why I added the label but didn't respond. maybe as you say, use the dot. Perhaps we show an orange dot for no vote, a green dot for a vote and show it for all wallets

@dated
Copy link
Contributor Author

dated commented Sep 22, 2019

👌 i'll try something in the next couple of days

@alexbarnsley
Copy link
Member

@dated did you get around to taking a look at this?

@dated
Copy link
Contributor Author

dated commented Sep 30, 2019

Nah, been busy with the plugin manager. I'll have a look this afternoon.

@dated
Copy link
Contributor Author

dated commented Sep 30, 2019

Instead of using colors (orange / green) I would propose to use an empty and filled circle instead.

@alexbarnsley
Copy link
Member

No worries! Yeh that sounds good 👌

@ghost ghost added the Status: Member Approved The pull request has been approved by a member. label Oct 15, 2019
@ghost
Copy link

ghost commented Oct 15, 2019

A member has approved this PR. A maintainer will merge this PR shortly. If it shouldn't be merged yet, please leave a comment saying so and we'll wait.

Thank you for your contribution!

@luciorubeens luciorubeens added the Bounty: Tier 3 Awarded for medium features, refactorings, improvements. This is valued at 50 USD. label Oct 15, 2019
@luciorubeens luciorubeens merged commit 77702d8 into ArkEcosystem:develop Oct 15, 2019
@ghost
Copy link

ghost commented Oct 15, 2019

Your pull request has been merged and marked as tier 3. It will earn you $50 USD.

@dated dated deleted the refactor/delegate-tab branch October 15, 2019 15:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bounty: Tier 3 Awarded for medium features, refactorings, improvements. This is valued at 50 USD. Complexity: Low Less than 64 lines changed. Status: Member Approved The pull request has been approved by a member. Status: Needs Changes The pull request needs additional changes before it can be merged. Type: Refactor The pull request improves or enhances an existing implementation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants