refactor(Notification): use Ant Design notifications for toasts #35222
Conversation
|
Based on your review schedule, I'll hold off on reviewing this PR until it's marked as ready for review. If you'd like me to take a look now, comment
|
msyavuz
changed the title
![korbit-ai[bot]](https://avatars.githubusercontent.com/in/322216?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review by Korbit AI
Korbit automatically attempts to detect when you fix issues in new commits.
| Category | Issue | Status |
|---|---|---|
 |
Unnecessary function wrappers ▹ view | ✅ Fix detected |
Files scanned
| File Path | Reviewed |
|---|---|
| superset-frontend/src/views/menu.tsx | ✅ |
| superset-frontend/src/views/RootContextProviders.tsx | ✅ |
| superset-frontend/src/embedded/EmbeddedContextProviders.tsx | ✅ |
| superset-frontend/src/components/MessageToasts/ToastContainer.tsx | ✅ |
| superset-frontend/src/components/MessageToasts/NotificationProvider.tsx | ✅ |
| superset-frontend/packages/superset-ui-core/src/components/index.ts | ✅ |
| superset-frontend/src/embedded/index.tsx | ✅ |
Explore our documentation to understand the languages and file types we support and the files we ignore.
Check out our docs on how you can make Korbit work best for you and your team.
| const value = useMemo<NotificationContextType>( | ||
| () => ({ | ||
| api, | ||
| success: args => api.success(args), | ||
| error: args => api.error(args), | ||
| warning: args => api.warning(args), | ||
| info: args => api.info(args), | ||
| open: args => api.open(args), | ||
| destroy: (key?: string) => api.destroy(key), | ||
| }), | ||
| [api], | ||
| ); |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Can we get rid of some of this extra padding at the bottom? |
|
Would be surprised if it looked this way in vanilla antd ... styles spilling over? |
|
I don't think its styles spilling over, |
msyavuz
force-pushed
the
msyavuz/fix/refactor-notifications
branch
from
84cc5b0 to
d200b24
Compare
|
Recent antd version causes a lot of test failures, i say we merge with the bottom padding now, it will be fixed as a byproduct when ant design gets upgraded. @rusackas |
Oh no! is it a major version? What seems to be the pattern behind these errors? |
|
Oops, since i force pushed all those are not visible on the pr now. https://github.com/apache/superset/actions/runs/17945544416?pr=35222
It's a minor version but that minor bump seems to render a bunch of things twice for some reason |
|
That's a bit concerning, I guess we'll have to deal with it at some point ... |
|
Whatever temporary fix we set for style we should TODO / comment around referencing the issue if there's one or PR comment above directly |
|
I will give upgrading ant design a go on this pr. You’re right that it’s something we’ll need to tackle sooner or later, so I might as well make life easier for future me |
SUMMARY
Replace the toast implementation with Ant Design notifications.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before:
After:
TESTING INSTRUCTIONS
Visual testing
ADDITIONAL INFORMATION