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] Notification types #6326

Open
klasjersevi opened this issue Sep 12, 2024 · 2 comments
Open

[FEAT] Notification types #6326

klasjersevi opened this issue Sep 12, 2024 · 2 comments
Labels
enhancement New feature or request wontfix This will not be worked on

Comments

@klasjersevi
Copy link

Is your feature request related to a problem? Please describe.

It's not possible to make a simple plain notification (in typescript) because useNotification requires type: "success" | "error" | "progress".

A plain notification would be needed when displaying a message or information.

It seems like the type restriction actually only is in the type definition.

Describe alternatives you've considered

Adding // @ts-expect-error type before the property makes it possible to override the value with any string. Omitting the type property or setting the type property to an empty string "" seems to make a simple plain notification. Setting the type to "warning" or "info" seems to work fine as well.

const { open } = useNotification();

open?.({
  message: "A plain notification",
  // @ts-expect-error type
  type: ""
})

Additional context

No response

Describe the thing to improve

Allow no or plain type as well as info and warning for notification options.

This can probably be done by extending the type of the type property to accept additional types, including no type.
It would be preferable if the type property also could be omitted to default to a plain notification.

interface OpenNotificationParams {
  key?: string;
  message: string;
+ type?: "" | "success" | "info" | "warning" | "error" | "progress";
- type: "success" | "error" | "progress";
  description?: string;
  cancelMutation?: () => void;
  undoableTimeout?: number;
}
@klasjersevi klasjersevi added the enhancement New feature or request label Sep 12, 2024
@aliemir
Copy link
Member

aliemir commented Sep 13, 2024

Hey @klasjersevi thank you for the issue! Refine's notificationProvider designed to handle notifications shown by Refine's hooks and components and those were the types we use in Refine. It will probably be better to use the notification library directly without using useNotification when working with custom types and shapes. Still, I think this is something we can support. We can make types success, error and progress typed and also allow any string as type to allow passing custom types through useNotification 🤔 As a result, its one less thing to worry about on the user side and one more thing Refine handles automatically 🎉

I'll try to send another comment soon to have a bit more detail about the implementation and someone can pick it up and work on it 🚀 Let us know if you want to work on this 🙏

Copy link

stale bot commented Nov 12, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix This will not be worked on label Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants