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

colw/Add link to buy Ledger Nano S in about page. #2663

Merged
merged 9 commits into from
May 29, 2019

Conversation

colw
Copy link
Contributor

@colw colw commented May 27, 2019

Description:

Add link to buy ledger nano S

Thank you! 🚀


For contributor:

  • Added changes entries. Run yarn changelog for a guided process.
  • Reviewed Files changed in the github PR explorer
  • Attach screenshots of the UI components on the PR description (if applicable)
  • Scope of work approved for big PRs

For reviewer:

  • Manually tested the changes on the UI

@colw colw requested review from faboweb and jbibla as code owners May 27, 2019 14:20
@colw colw changed the title Add link to buy Ledger Nano S in about page. colw/Add link to buy Ledger Nano S in about page. May 27, 2019
@colw colw force-pushed the colw/add-link-to-buy-ledger branch from cccd886 to 4e20d20 Compare May 27, 2019 14:22
@@ -43,7 +43,12 @@
<p>
To send transactions with Lunie, you'll have to sign them with your
Ledger Nano&nbsp;S. If you don't have a Ledger Nano S, you can
<a href="" target="_blank" rel="noopener norefferer">buy one here</a>.
<a
href="https://shop.ledger.com/products/ledger-nano-s"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
href="https://shop.ledger.com/products/ledger-nano-s"
href="https://shop.ledger.com/?r=3dd204ef7508"

Copy link
Contributor Author

@colw colw May 27, 2019

Choose a reason for hiding this comment

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

Should we just refer to the generic 'Ledger' or 'Ledger Nano`? (i.e. S, X or Blue)

Copy link
Collaborator

Choose a reason for hiding this comment

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

i think we can say "Ledger Nano" for now on - since we support S and X - what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure 👍

@jbibla
Copy link
Collaborator

jbibla commented May 27, 2019

@colw can you add the same link to these session screens?

"Don't have a Ledger yet? Get one here."

Screen Shot 2019-05-27 at 11 25 33 AM
Screen Shot 2019-05-27 at 11 25 41 AM

@colw
Copy link
Contributor Author

colw commented May 27, 2019

Top screen: Do you want it in addition to the ledger app message?

@colw
Copy link
Contributor Author

colw commented May 29, 2019

Screenshot 2019-05-29 at 16 40 45

Screenshot 2019-05-29 at 16 40 54

I also removed all references to Nano S in favour of Nano.

@colw
Copy link
Contributor Author

colw commented May 29, 2019

I could also remove he max-width: 320px on that top label?

@jbibla jbibla merged commit e7d177f into develop May 29, 2019
@jbibla jbibla deleted the colw/add-link-to-buy-ledger branch May 29, 2019 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants