goetzrobin / spartan

Cutting-edge tools powering Angular full-stack development.
https://spartan.ng
MIT License
1.22k stars 133 forks source link

feat(toast): Add toast component #135

Closed jstnjs closed 5 months ago

jstnjs commented 7 months ago

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

Which package are you modifying?

What is the current behavior?

Closes #

What is the new behavior?

Does this PR introduce a breaking change?

Other information

jstnjs commented 7 months ago

Hey @goetzrobin,

There is still lots to refactor, but I'm wondering about the styling. So brain shouldn't have any styles at all or just some basic styles? Right now there is a lot of basic styles in the brn-toaster.component, is that allowed or what should I do with it? Also should it be moved to tw classes as well? Or is this fine?

I'm thinking about a hlm directive on top of the toaster that adds the [data-styled]=true styling. I think the other styling could be staying in brain. What do you think?

goetzrobin commented 7 months ago

@jstnjs this is interesting. The idea was that brn components are unstyled by default. They never have a dependency in tailwind. I definitely would like to keep that no dependency on tailwind, but I'm not sure that unstyled by default makes sense in this context. Do you know what Sonner does?

jstnjs commented 7 months ago

@goetzrobin Hmm. The sonner has styling by default. There is a prop called unstyled where, if turned on, the toast itself does not have styling. You can add classes in the toastOptions object. But all other stying such as mentioned below, still exists.

Which is what we could seperate between brain and helm, but what about the default transitions, positions (top-left, bottom-right, etc), showing close or action buttons etc? I'm not sure if it makes sense to have it not be included in brain.

jstnjs commented 7 months ago

@goetzrobin Alright for the positions we could use overlayModule and for the default animations we could provide angular animations maybe? Is that allowed? Maybe this could work. For the action buttons we could use props and no styling, idk why I thought that would be a problem.

goetzrobin commented 6 months ago

@jstnjs just wanted to give you a heads up that we migrated to 17.1 to allow for signal input support! Also moved to the project to pnpm and avoid dependency issues with yarn!

goetzrobin commented 6 months ago

@goetzrobin Alright for the positions we could use overlayModule and for the default animations we could provide angular animations maybe? Is that allowed? Maybe this could work. For the action buttons we could use props and no styling, idk why I thought that would be a problem.

do you mean the Angular CDK Overlay? That's what I have been using and would recommend very much! It's supported by the Angular team, takes care of accessibility out of the box and is already a dependency in the project!