Copy backup button #1638

2018-06-19T04:31:13+02:00

Grayyyyy commented

2018-06-19 04:31:13 +02:00

(Migrated from github.com)

https://github.com/lbryio/lbry-app/issues/1635 completely implemented with this. Needs a bit of testing; worked on my machine.

neb-b commented

2018-06-19 08:12:46 +02:00

(Migrated from github.com)

Instead of creating a component specific backup copy, we should create a more generic component. This would be used by the address component, and on the backup page. Maybe something like this:

<CopyableInput
    inputText={lbryumWalletDir}
    snackbarText={__('Folder path copied')}
    showCopyButton
/>

Then we aren't duplicating very similar code accross the app.

Instead of creating a component specific backup copy, we should create a more generic component. This would be used by the address component, and on the backup page. Maybe something like this: ``` <CopyableInput inputText={lbryumWalletDir} snackbarText={__('Folder path copied')} showCopyButton /> ``` Then we aren't duplicating very similar code accross the app.

neb-b commented

2018-06-19 08:14:12 +02:00

(Migrated from github.com)

I closed the other PR since the code exists in this PR. But those comments apply to this one as well.

neb-b (Migrated from github.com) requested changes 2018-06-19 08:15:44 +02:00

neb-b (Migrated from github.com) left a comment

Lets make this component more generic so it can be used in several different places across the app.

Looking good though.

Lets make this component more generic so it can be used in several different places across the app. Looking good though.

Grayyyyy commented

2018-06-19 15:49:11 +02:00

(Migrated from github.com)

Alright, I'll try.

Grayyyyy commented

2018-06-20 02:31:24 +02:00

(Migrated from github.com)

Alright, I've created a new component called CopyableText. It's basically a universal component for making text boxes that are copyable, not just for Backup and Address. Clicking the copy button brings up a popup saying "Text copied" and copies the text box into the clipboard. Was this what you requested?

neb-b commented

2018-06-21 05:55:28 +02:00

(Migrated from github.com)

Yep that is what I was thinking. This would be used in the backup copy, shapeshift, and wallet address. Can you push up these changes so I can review them?

kauffj commented

2018-06-25 18:46:26 +02:00

(Migrated from github.com)

@Grayyyyy time for you to learn a new skill!

Can you rebase and squash these changes so that they come in as a single commit on top of the current master branch?

https://github.com/lbryio/lbry/wiki/Branching-and-Merging#rebase-onto-master-daily

(If you find this tricky, we can accept this as-is and do it ourselves, but I figured you'd want to learn)

@Grayyyyy time for you to learn a new skill! Can you `rebase` and `squash` these changes so that they come in as a single commit on top of the current `master` branch? https://github.com/lbryio/lbry/wiki/Branching-and-Merging#rebase-onto-master-daily (If you find this tricky, we can accept this as-is and do it ourselves, but I figured you'd want to learn)

neb-b (Migrated from github.com) requested changes 2018-06-27 08:05:56 +02:00

neb-b (Migrated from github.com) left a comment

One change, as well as Jeremy's suggestion about rebasing.

Sorry this took so long to get reviewed, we were all pretty swamped getting things ready for the redesign release.

The help page styling needs a little love. I will make some changes once this PR is ready to be merged.

One change, as well as Jeremy's suggestion about rebasing. Sorry this took so long to get reviewed, we were all pretty swamped getting things ready for the redesign release. The help page styling needs a little love. I will make some changes once this PR is ready to be merged.

src/renderer/component/copyableText/view.jsx

					
				@ -0,0 +44,4 @@

				          onClick={() => {

				            clipboard.writeText(copyable);

				            doNotify({

				              message: __('Text copied'),

neb-b (Migrated from github.com) commented

2018-06-27 08:04:15 +02:00

We can default to Text copied but we should be able to pass in any message to use. Ex: on the wallet overview/shapeshift, we should say Address copied.

We can default to `Text copied` but we should be able to pass in any message to use. Ex: on the wallet overview/shapeshift, we should say `Address copied`.

Grayyyyy commented

2018-06-27 19:56:11 +02:00

(Migrated from github.com)

Just rebased and squashed... I think. GitHub is saying "Changes requested", how do I approve the change you made to the code?

neb-b commented

2018-06-28 03:01:02 +02:00

(Migrated from github.com)

I have to approve it 🙂

Can you push up your changes after rebasing? You will have to do a force push

I have to approve it 🙂 Can you push up your changes after rebasing? You will have to do a force push

Grayyyyy commented

2018-06-28 03:13:21 +02:00

(Migrated from github.com)

Rebased and force pushed my changes.

Grayyyyy commented

2018-06-28 03:13:54 +02:00

(Migrated from github.com)

Do I need to add your code change to the code manually?

neb-b commented

2018-06-28 05:58:14 +02:00

(Migrated from github.com)

No I will push a commit to your PR. It looks like this component still doesn't accept a prop that changes the notification message. Could you add that? See my comment above.

Grayyyyy commented

2018-07-05 12:47:55 +02:00

(Migrated from github.com)

Hey, I don't know how to do this, since while I do know a bit of JavaScript, I don't know ReactJS. I couldn't figure out how to add the "custom message/prop" thing. Are there any resources/tips that would be helpful?

neb-b (Migrated from github.com) reviewed 2018-07-06 04:10:56 +02:00

src/renderer/component/shapeShift/internal/active-shift.jsx

neb-b (Migrated from github.com) commented

2018-07-06 04:10:56 +02:00

Just realized you are calling this prop copyable. A better name would be value.

Just realized you are calling this prop `copyable`. A better name would be `value`.

neb-b commented

2018-07-06 04:16:56 +02:00

(Migrated from github.com)

Hey @Grayyyyy

The official react docs are pretty good. https://reactjs.org/docs/components-and-props.html

This new prop notificationMessage will be passed in from the component that use CopyableText

It could work like this:

// address/view.jsx
  ...
  <CopyableText notificationMessage={__('Address copied')} value={walletAddress} />
  ...

Then in the component, you might have something like this:

// copyable-text.jsx
export function (props) => {
  const { doNotify, notificationMessage } = props;
  return (
    ...
    <Button onClick={() => doNotify({
         message: notificationMessage || __('Text copied'),
         displayType: ['snackbar'],
    )} />
    ...
  )
}

You are just passing in a new prop notificationMessage. So specific components that use this new component can edit what is displayed.

Hey @Grayyyyy The official react docs are pretty good. https://reactjs.org/docs/components-and-props.html This new prop `notificationMessage` will be passed in from the component that use `CopyableText` It could work like this: ``` // address/view.jsx ... <CopyableText notificationMessage={__('Address copied')} value={walletAddress} /> ... ``` Then in the component, you might have something like this: ``` // copyable-text.jsx export function (props) => { const { doNotify, notificationMessage } = props; return ( ... <Button onClick={() => doNotify({ message: notificationMessage || __('Text copied'), displayType: ['snackbar'], )} /> ... ) } ``` You are just passing in a new prop `notificationMessage`. So specific components that use this new component can edit what is displayed.

neb-b commented

2018-07-09 05:19:27 +02:00

(Migrated from github.com)

Just realized I commented midway through typing a sentence. Just completed my thought.

neb-b commented

2018-07-16 20:05:11 +02:00

(Migrated from github.com)

@Grayyyyy are you planning on finishing this up? Let me know if you have additional questions, maybe we could chat over discord.

Grayyyyy commented

2018-07-17 10:08:42 +02:00

(Migrated from github.com)

I am planning on finishing this up, and if I have any questions, I'll ping you on Discord 👍