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.

Sign up for GitHub

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

Jump to bottom

feat: Add toast #2772

Merged
merged 19 commits into from
Jan 15, 2021
Merged

feat: Add toast #2772

merged 19 commits into from
Jan 15, 2021

Conversation

h7y
Copy link
Member

@h7y h7y commented Jan 12, 2021 •
edited
Loading

A simple toast component that can display one toast at a time.

Fixes #2664

@vercel
Copy link

vercel bot commented Jan 12, 2021 •
edited
Loading

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/excalidraw/excalidraw/h67b267zp
✅ Preview: https://excalidraw-git-fork-h7y-add-toast.excalidraw.now.sh

lipis
lipis requested changes Jan 14, 2021
View reviewed changes
Copy link
Member

@lipis lipis left a comment

Choose a reason for hiding this comment

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

I think it would be better to show it in the bottom middle of the screen.. and instead of sliding it.. making it appear and disappear (opacity?). It should also stay awake if user has a mouse over it.. once the mouse is out or not there at all.. should go away.

@h7y
Copy link
Member Author

h7y commented Jan 14, 2021

I'll do it :) Are the colors and size fine?

@dwelle
Copy link
Member

dwelle commented Jan 14, 2021 •
edited
Loading

I think it should be much smaller. Something like what Google Drive is using?

image

Also, for this particular case of copying PNG to clipboard, make sure the toast is displayed only if the action succeeds.

@lipis
Copy link
Member

lipis commented Jan 14, 2021

Are the colors and size fine?

we can tweak as we go.

@lipis lipis mentioned this pull request Jan 14, 2021
Closed
@h7y
Copy link
Member Author

h7y commented Jan 14, 2021 •
edited
Loading

I've made the changes. Let me know if there are more changes!
Demo 2

@h7y h7y requested a review from lipis January 14, 2021 17:50
lipis
lipis reviewed Jan 15, 2021
View reviewed changes
src/locales/en.json Outdated Show resolved Hide resolved
lipis
lipis reviewed Jan 15, 2021
View reviewed changes
src/components/Toast.tsx Outdated Show resolved Hide resolved
lipis
lipis reviewed Jan 15, 2021
View reviewed changes
src/components/Toast.scss Outdated Show resolved Hide resolved
lipis
lipis reviewed Jan 15, 2021
View reviewed changes
src/components/Toast.scss Outdated Show resolved Hide resolved
lipis
lipis reviewed Jan 15, 2021
View reviewed changes
src/components/Toast.tsx Outdated Show resolved Hide resolved
@h7y h7y requested a review from lipis January 15, 2021 10:02
lipis
lipis reviewed Jan 15, 2021
View reviewed changes
src/components/Toast.scss Outdated Show resolved Hide resolved
@dwelle
Copy link
Member

dwelle commented Jan 15, 2021

Btw, we should make the toast use different style in dark-mode.

In fact, I'd align the style to what we use for the "scroll to content" message:

image

image

@h7y
Copy link
Member Author

h7y commented Jan 15, 2021

Updated toast

@lipis
Copy link
Member

lipis commented Jan 15, 2021

Nice one with the colors!

lipis
lipis reviewed Jan 15, 2021
View reviewed changes
src/components/Toast.scss Outdated Show resolved Hide resolved
@lipis
Copy link
Member

lipis commented Jan 15, 2021

What about adding that when copying styles as well?

@lipis
Copy link
Member

lipis commented Jan 15, 2021

But in general this is ready for merge.

@dwelle
Copy link
Member

dwelle commented Jan 15, 2021

I wouldn't display it during regular copy. You don't really need a confirmation for such a common action.

@lipis
Copy link
Member

lipis commented Jan 15, 2021

Not a regular copy of course.. Cmd+Option+C

@dwelle
Copy link
Member

dwelle commented Jan 15, 2021

I need to read more careful 😄

@h7y
Copy link
Member Author

h7y commented Jan 15, 2021

I'll also show on pasting styles?

@dwelle
Copy link
Member

dwelle commented Jan 15, 2021

No need on paste IMO — most of the time it should be obvious from the action itself.

lipis
lipis reviewed Jan 15, 2021
View reviewed changes
src/locales/en.json Outdated Show resolved Hide resolved
@dwelle
Copy link
Member

dwelle commented Jan 15, 2021

I've pushed a change that makes the Toast render conditionally, simplified the timer logic and handlers, and made the Toast reset the timer on mouseleave instead of hiding the toast immediately.

@dwelle
Copy link
Member

dwelle commented Jan 15, 2021

And finally, made exporting empty scene throw instead of alerting. This way the Toast isn't shown in this case. Throwing is also more correct.

With this, we're ready to merge.

@h7y
Copy link
Member Author

h7y commented Jan 15, 2021

I've pushed a change that makes the Toast render conditionally, simplified the timer logic and handlers, and made the Toast reset the timer on mouseleave instead of hiding the toast immediately.

Much cleaner!

@dwelle dwelle merged commit 543c624 into excalidraw:master Jan 15, 2021
lipis added a commit that referenced this pull request Jan 15, 2021
@lipis
Merge branch 'master' of github.com:excalidraw/excalidraw into cleanipp
1eb8920 
* 'master' of github.com:excalidraw/excalidraw: (37 commits)
  feat: Add toast (#2772)
  docs: Update readme with documentation (#2788)
  fix: allow text-selecting in dialogs & reset cursor (#2783)
  chore: Update translations from Crowdin (#2742)
  fix: broken Individuals link (#2782)
  refactor: Converting span to kbd tag (#2774)
  fix: don't render due to zoom after unmount (#2779)
  fix: Track the chart type correctly (#2773)
  chore(deps-dev): bump terser-webpack-plugin in /src/packages/excalidraw (#2750)
  chore(deps-dev): bump webpack in /src/packages/utils (#2768)
  fix: delay version logging & prevent duplicates (#2770)
  chore(deps-dev): bump webpack in /src/packages/excalidraw (#2769)
  chore(deps-dev): bump ts-loader in /src/packages/excalidraw (#2749)
  chore(deps-dev): bump ts-loader in /src/packages/utils (#2753)
  chore(deps): bump nanoid from 2.1.11 to 3.1.20 (#2581)
  feat: Track current version (#2731)
  chore(actions): Use cancel workflow action (#2763)
  chore(deps): bump @testing-library/react from 11.2.2 to 11.2.3 (#2755)
  chore(deps-dev): bump firebase-tools from 9.1.0 to 9.1.2 (#2761)
  chore(deps-dev): bump eslint-plugin-prettier from 3.3.0 to 3.3.1 (#2754)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lipis lipis lipis approved these changes

@dwelle dwelle dwelle approved these changes

@vjeux vjeux Awaiting requested review from vjeux

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Feedback when copying to clipboard
3 participants
@h7y @dwelle @lipis