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

feat: pending balance tooltip with unconfirmed transactions #1395

Merged
merged 15 commits into from
Sep 16, 2019
Merged

feat: pending balance tooltip with unconfirmed transactions #1395

merged 15 commits into from
Sep 16, 2019

Conversation

dated
Copy link
Contributor

@dated dated commented Aug 19, 2019 •
edited
Loading

This adds a tooltip to the balance in the wallet header showing the count and sum of unconfirmed transactions.

Changed to show the remaining balance including the unconfirmed transactions instead.

image

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 Aug 19, 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: Feature The issue is a request for new functionality. labels Aug 19, 2019
@alessiodf
Copy link
Contributor

over doesn't seem the right word to use in the tooltip imo. Maybe something like with a total of (or just totalling) or maybe accounting for would be better.

@alessiodf
Copy link
Contributor

You're welcome.

@dated dated changed the title feat: add tooltip showing sum of pending transactions feat: add tooltip showing pending balance including unconfirmed transactions Aug 20, 2019
@dated dated changed the title feat: add tooltip showing pending balance including unconfirmed transactions [WIP] feat: add tooltip showing pending balance including unconfirmed transactions Aug 20, 2019
@dated dated changed the title [WIP] feat: add tooltip showing pending balance including unconfirmed transactions feat: add tooltip showing pending balance including unconfirmed transactions Aug 20, 2019
@dated dated changed the title feat: add tooltip showing pending balance including unconfirmed transactions [WIP] feat: add tooltip showing pending balance including unconfirmed transactions Aug 20, 2019
@dated dated changed the title [WIP] feat: add tooltip showing pending balance including unconfirmed transactions feat: add tooltip showing pending balance including unconfirmed transactions Aug 20, 2019
@ghost ghost mentioned this pull request Aug 25, 2019
Closed
@ItsANameToo
Copy link
Member

Works, but 2 remarks:

  1. can you add a pointer: cursor when the tooltip is available and you hover over the amount?
  2. I hover over it as I know it exists, but I doubt a regular user will find this feature. So maybe we need a way to indicate that the amount can be hovered over when there is a pending transaction? 🤔 thoughts @alexbarnsley ?

@alexbarnsley
Copy link
Member

I agree, but not sure how you would highlight that it can be hovered over 🤔

@ItsANameToo
Copy link
Member

We could add a dotted underline to the amount or something? 🤔

@ItsANameToo
Copy link
Member

Only when the tooltip becomes available, so it would attract attention to it as it has "changed"

@luciorubeens
Copy link
Contributor

Maybe an icon next to the amount

@alexbarnsley
Copy link
Member

Maybe ask Oleg for an icon to highlight that there's a tooltip, something we can use (almost) anywhere

@ItsANameToo ItsANameToo added Status: Needs Changes The pull request needs additional changes before it can be merged. Status: Needs Discussion The issue or pull request needs more discussion before it can be closed or merged. labels Sep 2, 2019
@ghost
Copy link

ghost commented Sep 2, 2019

This issue or pull request needs further discussion. Please wait for further information, thank you.

@ghost
Copy link

ghost commented Sep 2, 2019

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

@faustbrian
Copy link
Contributor

@alexbarnsley @dated did Oleg provide an icon or was that ditched in favour of the pointer cursor and can be merged?

@dated
Copy link
Contributor Author

dated commented Sep 15, 2019 •
edited
Loading

I'm kinda waiting on further instructions :shrugging:

@faustbrian
Copy link
Contributor

@alexbarnsley what's needed for @dated to complete this PR?

@alexbarnsley
Copy link
Member

We can leave the icon for now

@ghost
Copy link

ghost commented Sep 16, 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!

@ghost ghost added the Status: Member Approved The pull request has been approved by a member. label Sep 16, 2019
@alexbarnsley alexbarnsley changed the title feat: add tooltip showing pending balance including unconfirmed transactions feat: pending balance tooltip with unconfirmed transactions Sep 16, 2019
@alexbarnsley alexbarnsley merged commit ebc44a9 into ArkEcosystem:develop Sep 16, 2019
@ghost
Copy link

ghost commented Sep 16, 2019

Your pull request has been merged but was not assigned a bounty tier. A maintainer will assign a bounty tier to this pull request in the next few days.

@ghost ghost removed the Status: Needs Discussion The issue or pull request needs more discussion before it can be closed or merged. label Sep 16, 2019
@dated dated deleted the feat/pending-amount branch September 16, 2019 09:07
@ghost ghost mentioned this pull request Sep 22, 2019
Closed
@alexbarnsley alexbarnsley added the Bounty: Tier 4 Awarded for small features, refactorings, improvements. This is valued at 20 USD. label Oct 1, 2019
@ghost
Copy link

ghost commented Oct 1, 2019

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@alexbarnsley alexbarnsley alexbarnsley approved these changes

@faustbrian faustbrian Awaiting requested review from faustbrian

@ItsANameToo ItsANameToo Awaiting requested review from ItsANameToo ItsANameToo is a code owner

@luciorubeens luciorubeens Awaiting requested review from luciorubeens

Assignees
No one assigned
Labels
Bounty: Tier 4 Awarded for small features, refactorings, improvements. This is valued at 20 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: Feature The issue is a request for new functionality.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@dated @alessiodf @ItsANameToo @alexbarnsley @luciorubeens @faustbrian