Add basic Toast component

willm0602 commented 2 months ago

Adds a basic version of a toast component that wraps Notifications and includes them on the bottom

Wanted to also leave a couple of notes before this gets merged

I wasn't able to find any tests for components specifically in the codebase. Is there anywhere I can add these before we merge these?
When I was testing this out, I wasn't able to initially since I got an issue, 'TZ' is not recognized as an internal or external command, operable program or batch file. I modified the test script in package.json to be SET TZ=UTC+4 vitest and that let the test suite run, but failed on six-issues, not sure if it's an issue with developing on windows OS

chrome_7oGfLfmPJR

stackblitz[bot] commented 2 months ago

Run & review this pull request in StackBlitz Codeflow.

changeset-bot[bot] commented 2 months ago

🦋 Changeset detected

Latest commit: e26bfa3ff4e00ca7c0165ac5196c8f46d61efa70

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

| Name | Type | | --------- | ----- | | svelte-ux | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions[bot] commented 2 months ago

built with Refined Cloudflare Pages Action

⚡ Cloudflare Pages Deployment

Name	Status	Preview	Last Commit
svelte-ux	✅ Ready (View Log)	Visit Preview	e26bfa3ff4e00ca7c0165ac5196c8f46d61efa70

techniq commented 2 months ago

I wasn't able to find any tests for components specifically in the codebase. Is there anywhere I can add these before we merge these?

I currently only have vitest setup for unit tests (mostly utils, example format.test.ts).

I would like to setup playwright for integration/browser tests and integrated into the CI pipeline, similar to shadcn-svelte and melt-ui's setup, but haven't had the time to tackle it yet. If that is an initiative you would like to get started, I would kindly review a PR 😁

When I was testing this out, I wasn't able to initially since I got an issue, 'TZ' is not recognized as an internal or external command, operable program or batch file. I modified the test script in package.json to be SET TZ=UTC+4 vitest and that let the test suite run, but failed on six-issues, not sure if it's an issue with developing on windows OS

Sadly most of the development occurs on Mac and thus a unix environment (and usually zsh or bash terminal). With that said, one of our contributors was using Windows and leveraging the WSL (he actually was the one to add the timezone environment variable, although he also switched to Mac later on).

I don't hear of much Windows open source development happening without using WSL nowadays, but I'm also pretty detached from that ecosystem (switched to Mac 10+ years ago).

If you're not using WSL, I would recommend setting it up and see if you have a better experience.

techniq commented 2 months ago

Few other ideas I forgot to mention. It might make sense to put an instance of the toastStore in settings() like we do for themes, locales, nav drawer, etc. We could also consider adding an instance of <Toast> within <AppLayout> wired up to this instance.

willm0602 commented 2 months ago

@techniq started working on the changes and got an issue where I can't get the toasters to appear over the elements of the app layout that used fixed position (so the side and topnav) even after adjusting the z-indexes

Was wondering if you had any ideas for fixing this?

techniq commented 2 months ago

@techniq started working on the changes and got an issue where I can't get the toasters to appear over the elements of the app layout that used fixed position (so the side and topnav) even after adjusting the z-indexes

Was wondering if you had any ideas for fixing this?

Modal elements like Drawer and Dialog "portal" their content to the bottom of <body> via the portal action and doing similar within Notification should solve the issue.

techniq commented 1 month ago

@willm0602 I started work on improving/simplifying the Notification component by support props (not requiring but still supporting slots) such as <Notification title="..." description="..." actions={...}> and also supporting color/variant/etc props. Take a look at the PR preview.

I also have a PR to use @layerstack/* packages for most all svelte stores/actions/etc. I'll plan to merge these hopefully in the next few days (the later is a much bigger change).

This will impact your PR some, but not sure where toastStore should live just yet (it's rather tied to Svelte UX Toast/Notification and not sure it makes much sense to be within @layerstack/*. In some ways I think of it similar to AppBar/AppLayout/NavItem and their use of showDrawer within settings.

techniq / svelte-ux