[RHCLOUD-33811] Move notification drawer to its own module #661
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #661 +/- ##
=======================================
Coverage 91.30% 91.30%
=======================================
Files 3 3
Lines 23 23
=======================================
Hits 21 21
Misses 2 2 |
| export const notificationDrawerDataAtom = atom<NotificationData[]>([]); | ||
| export const notificationDrawerCountAtom = atom(0); |
There was a problem hiding this comment.
I think the initial data should be passed from chrome as well as props. The notifications module should only consume them.
config/webpack.cy.config.js
Outdated
There was a problem hiding this comment.
This PR should not add cypress config. We have a seperate ticket for that work.
There was a problem hiding this comment.
The PR did get big pretty quick haha. I threw these (imperfect) cypress stuff in a draft PR for you all to start with #667
| key="manage-event" | ||
| onClick={() => | ||
| onNavigateTo( | ||
| `/settings/notifications/configure-events?bundle=${notification.bundle}&tab=configuration` |
There was a problem hiding this comment.
Since we are in the notifications application I think all links needs to be updated and the /settings should be removed. I would have to however have to check it locally what needs to remain and what can be removed.
| <DropdownItem key="quick links" description="Quick links" />, | ||
| <DropdownItem | ||
| key="notifications log" | ||
| onClick={() => onNavigateTo('/settings/notifications/notificationslog')} |
There was a problem hiding this comment.
Same as with the to links. I think we can remove the /settings URL. But I'd have to check this locally to see what can remain and what can be removed.
| const onMarkAsRead = () => { | ||
| axios | ||
| .put('/api/notifications/v1/notifications/drawer/read', { | ||
| notification_ids: [notification.id], | ||
| read_status: !notification.read, | ||
| }) | ||
| .then(() => { | ||
| updateNotificationRead(notification.id, !notification.read); | ||
| setIsDropdownOpen(false); | ||
| }) | ||
| .catch((e) => { | ||
| console.error('failed to update notification read status', e); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Let's use the JS client for this function as well.
Description
Creates a new component for the NotificationDrawer. The functionality was previously in the insights-chrome
repo and will now live here and be called as a
ScalprumComponentRHCLOUD-33811
Checklist ☑️