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

Display QrCode for accounts, addresses & contracts #4329

Merged
merged 3 commits into from
Jan 27, 2017
Merged

Conversation

jacogr
Copy link
Contributor

@jacogr jacogr commented Jan 27, 2017

QrCode verified scannable and returning the correct address via iOS Ethereum wallet.

parity 2017-01-27 14-17-14

@jacogr jacogr added A0-pleasereview 🤓 Pull request needs code review. M7-ui labels Jan 27, 2017
@jacogr jacogr changed the title Display scannable QrCode for accounts, addresses & contracts Display QrCode for accounts, addresses & contracts Jan 27, 2017
@ngotchac
Copy link
Contributor

It looks really unbalanced to me. Can't the tokens still be right bellow the name/address ? Might want to have a smaller QR Code too, with some opacity, that could grow and be opaque on hover, no ? Right now it looks like it's the main info

@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

Also, don't we want to wait for https://github.com/ethcore/parity/issues/4267 so we can directly embed links to the address ?

@jacogr
Copy link
Contributor Author

jacogr commented Jan 27, 2017

Wouldn't make this dependent on #4267 at all, just going to blow up for no reason. Could definately squish down the size a bit and see how the tokens flow.

@ngotchac
Copy link
Contributor

@jacogr Ok. And I didn't mean dependant, but maybe just wait for use to support this web+ethereum:... pattern and use it there. Not sure if it's what we want for the QR code, but if it is, then changing from plain address to URL might be misleading, no ?

@jacogr
Copy link
Contributor Author

jacogr commented Jan 27, 2017

QrCode is there for scanning first and foremost. So I want to scan the address on my iOS app and send it ETH. (And once we have this https://github.com/ethcore/parity/issues/3075 in, we will add it to the signer as well)

@ngotchac
Copy link
Contributor

But isn't it the point of scanning ? So that the mobile phone can trigger the right app to send a payment without having to copy-paste ?

@jacogr
Copy link
Contributor Author

jacogr commented Jan 27, 2017

Taking this to Slack since I we are missing each other here.

@ngotchac
Copy link
Contributor

My bad, misunderstanding... Just waiting on UI changes then

@jacogr jacogr added A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. and removed A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Jan 27, 2017
@jacogr
Copy link
Contributor Author

jacogr commented Jan 27, 2017

Updated. Ended up not converting the layouts to use flex within flex here (something on left, something on right), sticking to the float right for the code - although there is less floating going on now with some simplification of the initial.

parity 2017-01-27 14-12-26

@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
@ngotchac ngotchac added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jan 27, 2017
@jacogr jacogr merged commit aef699e into master Jan 27, 2017
@jacogr jacogr deleted the jg-qrcode-addresses branch January 27, 2017 20:55
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.

2 participants