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

fix: add margin only when confirmations truthy & chores #1145

Merged
merged 6 commits into from Mar 26, 2019

Conversation

@dated
Copy link
Contributor

commented Mar 20, 2019

Proposed changes

Additional changes to #1129 which can either be merged into that branch or I'll rebase this PR if #1129 gets merged first.

This PR adds a clipboard button for the block id and adds a condition to the clipboard button of the transaction id, so there is no right margin when the explorer link is missing.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • Other... Please describe: chores

Checklist

  • I have read the CONTRIBUTING documentation
  • Lint and unit tests pass locally with my changes
@ArkEcosystemBot

This comment has been minimized.

Copy link
Member

commented Mar 20, 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.

@j-a-m-l

This comment has been minimized.

Copy link
Contributor

commented Mar 20, 2019

Thanks @dated. I'll wait until the other PR is merged to review this one.

@dated

This comment has been minimized.

Copy link
Contributor Author

commented Mar 20, 2019

👍 I'll rebase this when it's time to do so

@dated dated changed the base branch from sha256-block-ids to develop Mar 20, 2019

@dated dated changed the base branch from develop to sha256-block-ids Mar 20, 2019

@dated dated force-pushed the dated:block-link branch from 75dc7ef to ec47510 Mar 20, 2019

@ArkEcosystemBot

This comment has been minimized.

Copy link
Member

commented Mar 21, 2019

Your pull request has been closed, thank you for trying to solve an issue. If you think it was closed prematurely please provide additional information.

@dated dated changed the base branch from sha256-block-ids to develop Mar 21, 2019

@dated

This comment has been minimized.

Copy link
Contributor Author

commented Mar 21, 2019

@alexbarnsley can you reopen? It was probably closed because the base got deleted.

@alexbarnsley alexbarnsley reopened this Mar 21, 2019

@ArkEcosystemBot

This comment has been minimized.

Copy link
Member

commented Mar 21, 2019

This pull-request has now been re-opened. If applicable please provide additional information as requested by one of the reviewers.

@alexbarnsley

This comment has been minimized.

Copy link
Member

commented Mar 21, 2019

Oops! Didn't realise it was linked to a different branch

@j-a-m-l

This comment has been minimized.

Copy link
Contributor

commented Mar 21, 2019

@dated could you clean the unrelated changes / commits?

Edited: sorry @danielstc for the mention

@dated

This comment has been minimized.

Copy link
Contributor Author

commented Mar 21, 2019

Rebasing must have messed that up. I'll fix it tonight.

dated added some commits Mar 20, 2019

@dated dated force-pushed the dated:block-link branch from 4c446a7 to 7a368f7 Mar 21, 2019

@dated

This comment has been minimized.

Copy link
Contributor Author

commented Mar 21, 2019

All done @j-a-m-l

@j-a-m-l
Copy link
Contributor

left a comment

The button to copy the block id is misaligned.

@dated

This comment has been minimized.

Copy link
Contributor Author

commented Mar 22, 2019

Sorry, that slipped by while adding the changes. Fixed now.

@j-a-m-l
Copy link
Contributor

left a comment

I've just tried it and the new transaction ID and the button were together, without a gap between them.

@j-a-m-l
Copy link
Contributor

left a comment

This is failing yet:

  1. Go to a wallet
  2. Send a transaction
  3. Refresh the transactions
  4. Click on the transaction
  5. If the transaction has 0 confirmations, the block ID is hidden (good), but the transaction ID isn't separated from the explorer link button.
@dated

This comment has been minimized.

Copy link
Contributor Author

commented Mar 26, 2019

image

Works here 🤔

@ArkEcosystemBot

This comment has been minimized.

Copy link
Member

commented Mar 26, 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!

@j-a-m-l j-a-m-l merged commit 6edf9d2 into ArkEcosystem:develop Mar 26, 2019

1 check passed

ci/circleci: test-node-11 Your tests passed on CircleCI!
Details
@ArkEcosystemBot

This comment has been minimized.

Copy link
Member

commented Mar 26, 2019

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

@j-a-m-l

This comment has been minimized.

Copy link
Contributor

commented Mar 26, 2019

@dated yesterday my branch wasn't correctly updated. I've tried again and it worked. Thanks.

@dated dated deleted the dated:block-link branch Mar 26, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.