Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Add QrCode & Copy to ShapeShift #4322

Merged
merged 7 commits into from
Jan 27, 2017
Merged

Add QrCode & Copy to ShapeShift #4322

merged 7 commits into from
Jan 27, 2017

Conversation

jacogr
Copy link
Contributor

@jacogr jacogr commented Jan 27, 2017

Closes https://github.com/ethcore/parity/issues/4238

QrCode output verified with the BlockChain.info app, able to read & translate.

parity 2017-01-27 10-10-04

@jacogr jacogr added A0-pleasereview 🤓 Pull request needs code review. M7-ui labels Jan 27, 2017
@@ -61,9 +63,7 @@ export default class AwaitingDepositStep extends Component {
} }
/>
</div>
<div className={ styles.hero }>
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the class .hero still used anywhere ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeap. Both in AwaitingExchange & CompletedStep.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah right, wasn't looking in the right folder. My bad !

@ngotchac ngotchac added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jan 27, 2017
@ngotchac
Copy link
Contributor

ngotchac commented Jan 27, 2017

Same comment as for #4329 : don't we want to use bitcoin:... URLs (if BTC transfer) ?

@jacogr
Copy link
Contributor Author

jacogr commented Jan 27, 2017

Could make it clickable with bitcoin:, good point.

@jacogr
Copy link
Contributor Author

jacogr commented Jan 27, 2017

Added bitcoin: clicky (nice, it works with my exchange, makes my life easier) and made the default size smaller.

parity 2017-01-27 13-19-09

@jacogr jacogr added A0-pleasereview 🤓 Pull request needs code review. and removed A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Jan 27, 2017
return (
<div className={ styles.addressInfo }>
<a
href={ `bitcoin:${depositAddress}` }
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be only for BTC transfers ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

&^%$! So excited to have it working that I didn't pay attention.

@ngotchac ngotchac added A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jan 27, 2017
@jacogr jacogr added A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. and removed A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. labels Jan 27, 2017
@jacogr jacogr added A0-pleasereview 🤓 Pull request needs code review. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels Jan 27, 2017

return component;
}

describe('modals/Shapeshift/AwaitingDepositStep', () => {
describe.only('modals/Shapeshift/AwaitingDepositStep', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Might want to get rid of this one ;)

@ngotchac ngotchac added A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jan 27, 2017
@jacogr jacogr added A0-pleasereview 🤓 Pull request needs code review. and removed A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. labels Jan 27, 2017
expect(qr.props().value).to.equal(TEST_ADDRESS);
});

describe('protocol link', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is under QrCode test the right place for these tests ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The link wraps the QrCode. Not attached to it either way, could be outside as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok

@ngotchac ngotchac added A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. labels Jan 27, 2017
@jacogr jacogr merged commit cb8fea3 into master Jan 27, 2017
@jacogr jacogr deleted the jg-qr-shapeshift branch January 27, 2017 14:33
jacogr added a commit that referenced this pull request Feb 3, 2017
* Extract CopyIcon to ~/ui/Icons

* Add copy & QrCode address

* Default size 4

* Add bitcoin: link

* use protocol links applicable to coin exchanged

* Remove .only
gavofyork pushed a commit that referenced this pull request Feb 4, 2017
* s/Delete Contract/Forget Contract/ (#4237)

* Adjust the location of the signer snippet (#4155)

* Additional building-block UI components (#4239)

* Currency WIP

* Expand tests

* Pass className

* Add QrCode

* Export new components in ~/ui

* s/this.props.netSymbol/netSymbol/

* Fix import case

* ui/SectionList component (#4292)

* array chunking utility

* add SectionList component

* Add TODOs to indicate possible future work

* Add missing overlay style (as used in dapps at present)

* Add a Playground for the UI Components (#4301)

* Playground // WIP

* Linting

* Add Examples with code

* CSS Linting

* Linting

* Add Connected Currency Symbol

* 2015-2017

* 2015-2017

* 2015-2017

* 2015-2017

* 2015-2017

* 2015-2017

* 2015-2017

* Added `renderSymbol` tests

* PR grumbles

* Add Eth and Btc QRCode examples

* 2015-2017

* Add tests for playground

* Fixing tests

* Split Dapp icon into ui/DappIcon (#4308)

* Add QrCode & Copy to ShapeShift (#4322)

* Extract CopyIcon to ~/ui/Icons

* Add copy & QrCode address

* Default size 4

* Add bitcoin: link

* use protocol links applicable to coin exchanged

* Remove .only

* Display QrCode for accounts, addresses & contracts (#4329)

* Allow Portal to be used as top-level modal (#4338)

* Portal

* Allow Portal to be used in as both top-level and popover

* modal/popover variable naming

* export Portal in ~/ui

* Properly handle optional onKeyDown

* Add simple Playground Example

* Add proper event listener to Portal (#4359)

* Display AccountCard name via IdentityName (#4235)

* Fix signing (#4363)

* Dapp Account Selection & Defaults (#4355)

* Add parity_defaultAccount RPC (with subscription) (#4383)

* Default Account selector in Signer overlay (#4375)

* Typo, fixes #4271 (#4391)

* Fix ParityBar account selection overflows (#4405)

* Available Dapp selection alignment with Permissions (Portal) (#4374)

* registry dapp: make lookup use lower case (#4409)

* Dapps use defaultAccount instead of own selectors (#4386)

* Poll for defaultAccount to update dapp & overlay subscriptions (#4417)

* Poll for defaultAccount (Fixes #4413)

* Fix nextTimeout on catch

* Store timers

* Re-enable default updates on change detection

* Add block & timestamp conditions to Signer (#4411)

* Extension installation overlay (#4423)

* Extension installation overlay

* Pr gumbles

* Spelling

* Update Chrome URL

* Fix for non-included jsonrpc

* Extend Portal component (as per Modal) #4392
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants