update toast #517
update toast #517
Conversation
🦋 Changeset detectedLatest commit: 232c665 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
@felicio is attempting to deploy a commit to the Status Team on Vercel. A member of the Team first needs to authorize it. |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
I understand the necessity to expand the component API to accommodate other needs. However, I don't think that consumers need to know about implementation details such as actionProps or rootProps.
I still think that keeping the first parameter as message and the second as options is cleaner way. Then it can be destructured and passed to appropriate components.
| options?: { | ||
| actionProps?: Pick<ToastProps, 'action' | 'onAction'> | ||
| rootProps?: ToastRootProps | ||
| } |
There was a problem hiding this comment.
type Options = ToastRootProps & Pick<ToastProps, 'action' | 'onAction'>There was a problem hiding this comment.
isn't it clearer that way? wouldn't destructuring for rootProps instead of its individual fields better ensure it's not passed further down where not expected?
There was a problem hiding this comment.
From a component consumer view, I don't see why they'd need to understand what root and action are.
The toast API
toast.success("Yaaayy!!", {
duration: 3000,
action: "Undo",
onAction: () => void
})is much cleaner than nesting it under some arbitrary fields.
There was a problem hiding this comment.
which option goes where is an implementational detail.
There was a problem hiding this comment.
The component handling the opening of the toast should be explicit about things a user can configure and pass it to the appropriate components (no spreading of props).
why