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

feat: add toasts #23

Merged
merged 5 commits into from
May 18, 2023
Merged

feat: add toasts #23

merged 5 commits into from
May 18, 2023

Conversation

alefDev-prog
Copy link
Contributor

fixes #10

Affected files:

  • src/app/page.tsx
  • src/utils/clipboard.ts

What I have done:

  1. Installed npm package react-toastify
  2. Inserted the ToastContainer in page.tsx
  3. Call toast-functions in clipboard.ts on fail and success

Issue I have encountered:

I have encountered an issue in src/components/ClipboardImage.tsx. The component in this file re-renders multiple times when clicking on the picture in the frame. This causes the pasteImage function to be called multiple times, resulting in several toasts apperaing on a single click. I think that the issue is with the imageCallback function and how it uses the useCallback hook. The items in its dependency-array are being changed within the function itself, something I think causes this issue.

I have tried to solve this issue without changing the useCallback hook but without success. As I wrote before, this results in multiple toasts appearing on a single click.

@vercel
Copy link

vercel bot commented May 17, 2023

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

Name Status Preview Comments Updated (UTC)
screenshot ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 17, 2023 4:39pm

Copy link
Collaborator

@iamhectorsosa iamhectorsosa left a comment

Choose a reason for hiding this comment

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

Hey, @alefDev-prog . This issue could be definitely cause by this.

However, before anything. I'd like to focus on implementing the toasts following: https://ui.shadcn.com/docs/components/toast

We can use the manual installation and copy the entire implementation, just make sure to adapt and use TailwindCSS colors.

This is the main AC from this issue. Once we've implemented the toasts to fit the UI we can then figure out what refactoring do we need in the component itself.

Since it is a bit of refactoring needed, I can take over this one if you'd need. I'm not happy with react-toastify package because we're using Radix for the most part and it falls outside of the UI aesthetic.

@alefDev-prog
Copy link
Contributor Author

Hi, ok sure I use your prefered package instead.

@iamhectorsosa
Copy link
Collaborator

We're heavily inspiring our look from that library so that's why I'm pushing for consistency. Sorry, I appreciate your help. It's just going to make it easier for all of us. @alefDev-prog

feat: added toasts

feat: added toasts without styling

feat: added toast without styling
@alefDev-prog
Copy link
Contributor Author

Hi, I understand that you want consistency. I have now included the package you wanted and I have set it up so that the toast gets presented to the user on fail or success. However, as I am not very experienced in Tailwind css I will leave the styling for someone else to do.

@iamhectorsosa
Copy link
Collaborator

@alefDev-prog good work here! Unfortunately, I cannot collaborate in this PR because you didn't use a branch and are working off main. In the future to ensure collaboration please from your forked repository open a branch.

When contributors fork your repository and intend to make changes or additions, it is generally recommended for them to create a separate branch in their forked repository. They should make their changes on that branch and then open a pull request (PR) from their branch to the main branch of your repository

@iamhectorsosa iamhectorsosa merged commit e81543c into webscopeio:main May 18, 2023
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.

Integrate Toasts
2 participants