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

wallet-ext: copy to clipboard use toast for confirmation feedback #7583

Merged
merged 1 commit into from
Jan 27, 2023

Conversation

pchrysochoidis
Copy link
Contributor

  • show a toast instead of scaling the copy icon
  • move copy to functionality to hooks to reuse it
  • also removes unused code for rendering sui objects

before

Screen.Recording.2023-01-23.at.12.14.26.mov

after

Screen.Recording.2023-01-23.at.12.11.12.mov
Screen.Recording.2023-01-23.at.12.11.34.mov

part of APPS-296

@vercel
Copy link

vercel bot commented Jan 23, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

4 Ignored Deployments
Name Status Preview Comments Updated
explorer ⬜️ Ignored (Inspect) Jan 27, 2023 at 2:07PM (UTC)
explorer-storybook ⬜️ Ignored (Inspect) Jan 27, 2023 at 2:07PM (UTC)
frenemies ⬜️ Ignored (Inspect) Jan 27, 2023 at 2:07PM (UTC)
wallet-adapter ⬜️ Ignored (Inspect) Jan 27, 2023 at 2:07PM (UTC)

};

export function useCopyToClipboard(
txpToCopy: string,
Copy link
Contributor

Choose a reason for hiding this comment

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

txtToCopy ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks renamed it

@mystie711
Copy link
Collaborator

Is the toast height correct?
Feels a tad bit taller? 🤔

@pchrysochoidis pchrysochoidis force-pushed the pc-wallet-ext-apps-295-ui-remove-keypair branch from 8910aaa to 3c29e5d Compare January 24, 2023 11:32
@pchrysochoidis pchrysochoidis force-pushed the pc-wallet-ext-apps-296-home-account-switch branch 2 times, most recently from 064ab94 to e5c4199 Compare January 24, 2023 11:47
@pchrysochoidis
Copy link
Contributor Author

Is the toast height correct? Feels a tad bit taller? 🤔

Should be the same as it already is in main, I did't change it.

@mystie711
Copy link
Collaborator

Is the toast height correct? Feels a tad bit taller? 🤔

Should be the same as it already is in main, I did't change it.

Could you please double check if we are using the correct size in main?

@pchrysochoidis
Copy link
Contributor Author

pchrysochoidis commented Jan 26, 2023

Is the toast height correct? Feels a tad bit taller? 🤔

Should be the same as it already is in main, I did't change it.

Could you please double check if we are using the correct size in main?

There was some extra margin from the lib, addressed here

Jibz1
Jibz1 previously approved these changes Jan 26, 2023
pchrysochoidis added a commit that referenced this pull request Jan 26, 2023
| before | after |
| - | - |
| <img width="417" alt="Screenshot 2023-01-26 at 18 13 51"
src="https://user-images.githubusercontent.com/10210143/214917095-59a744a2-d728-48db-875f-a41f93a52f01.png">
| <img width="417" alt="Screenshot 2023-01-26 at 18 14 05"
src="https://user-images.githubusercontent.com/10210143/214917146-6999f18b-95fc-48da-bf04-e5fc45ee1afa.png">
|

addresses
#7583 (comment)
@pchrysochoidis pchrysochoidis force-pushed the pc-wallet-ext-apps-296-home-account-switch branch from e5c4199 to f69dd7e Compare January 27, 2023 13:08
@pchrysochoidis pchrysochoidis changed the base branch from pc-wallet-ext-apps-295-ui-remove-keypair to main January 27, 2023 13:08
@pchrysochoidis pchrysochoidis dismissed Jibz1’s stale review January 27, 2023 13:08

The base branch was changed.

return (
<span
className={cl(st.container, className)}
onClick={!copyOnlyOnIconClick ? copyToClipboard : undefined}
>
{children}
<Icon
className={cl(st.copyIcon, st[mode], { [st.copied]: copied })}
className={cl(st.copyIcon, st[mode])}
Copy link
Contributor

Choose a reason for hiding this comment

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

can we use Tailwind here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can but this component has other usages of css modules and classes and I think it's better to address them in an another PR that will do only that

* show a toast instead of scaling the copy icon
* move copy to functionality to hooks to reuse it
* also removes unused code for rendering sui objects
@pchrysochoidis pchrysochoidis force-pushed the pc-wallet-ext-apps-296-home-account-switch branch from f69dd7e to c84fb26 Compare January 27, 2023 14:06
@pchrysochoidis pchrysochoidis merged commit 98f6906 into main Jan 27, 2023
@pchrysochoidis pchrysochoidis deleted the pc-wallet-ext-apps-296-home-account-switch branch January 27, 2023 14:24
williampsmith pushed a commit that referenced this pull request Feb 3, 2023
| before | after |
| - | - |
| <img width="417" alt="Screenshot 2023-01-26 at 18 13 51"
src="https://user-images.githubusercontent.com/10210143/214917095-59a744a2-d728-48db-875f-a41f93a52f01.png">
| <img width="417" alt="Screenshot 2023-01-26 at 18 14 05"
src="https://user-images.githubusercontent.com/10210143/214917146-6999f18b-95fc-48da-bf04-e5fc45ee1afa.png">
|

addresses
#7583 (comment)
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.

4 participants