Notification Drawer Update #2571

arivepr · 2023-07-17T07:03:53Z

Addressing the following JIRA card
Note: This PR nor card are intended to address getting real data in. This is purely UI based for now with dummy data hard-coded in.

arivepr · 2023-07-26T18:37:09Z

Note
Unit testing seems to be failing, though not entirely sure how to fix what it's complaining about (not something i touched in this PR)
Additionally, had to comment out some of the useFlags as they weren't being used according to my lint check, left them there just in case rather than outright deleting. Just let me know how you'd like me to handle those @karelhala

:)

karelhala

Looking good! A couple of changes and we'll be golden!

package-lock.json

src/components/Header/Tools.tsx

src/components/NotificationsDrawer/DrawerPanelContent.tsx

src/redux/action-types.ts

src/redux/chromeReducers.ts

ryelo · 2023-08-17T12:16:49Z

src/redux/notificationTestData.js

Can we update these tests to reflect what actual notifications would look like?

Well im taking those out once i get the green light on everything else (just to save time), but if we have live examples somewhere I'm happy to replace them for screenshots or what not

arivepr · 2023-08-18T19:47:54Z

Should be good to go now, everything tested before last commit.
I propose refactoring the header for the drawer panel into its own subcomponent just to keep things tidy, but I can do that in a separate PR after this gets connected to actual data.

Once the green light is given for the overall changes, test data will be taken out

karelhala

Looking hella good! Just a few nitpicks and we are golden!

src/components/NotificationsDrawer/DrawerPanelContent.tsx

src/components/NotificationsDrawer/NotificationItem.tsx

codecov-commenter · 2023-08-22T18:10:45Z

Codecov Report

Merging #2571 (9dd2ce6) into master (0b9357a) will decrease coverage by 0.20%.
The diff coverage is 40.62%.

❗ Current head 9dd2ce6 differs from pull request most recent head 668c93f. Consider uploading reports for the commit 668c93f to get more accurate results

@@            Coverage Diff             @@
##           master    #2571      +/-   ##
==========================================
- Coverage   57.38%   57.19%   -0.20%     
==========================================
  Files          91       92       +1     
  Lines        2783     2815      +32     
  Branches      648      581      -67     
==========================================
+ Hits         1597     1610      +13     
- Misses       1111     1204      +93     
+ Partials       75        1      -74

Files Changed	Coverage
src/redux/index.ts	`ø`
src/redux/chromeReducers.ts	`17.39%`
...nts/NotificationsDrawer/notificationDrawerUtils.ts	`100.00%`
src/redux/action-types.ts	`100.00%`
src/redux/actions.ts	`100.00%`

arivepr · 2023-08-22T18:14:20Z

I'm seeing a bunch of new errors after rebasing, trying to figure out what that all is about.

karelhala

Let's use lodash's sortBy instead of manual sort to have proper sorting and once the commented out feature flag options are removed and eslint if fixed we can merge it!

src/components/NotificationsDrawer/DrawerPanelContent.tsx

arivepr · 2023-08-29T17:12:17Z

~~Switching to lodash now per request, just btw, we do have proper sorting as is 😉 😆~~

I take it back, this is way better/easier lol

karelhala

Looking good! Just nitpick about DateFormat and importing testing data, but overall looks good

src/components/NotificationsDrawer/NotificationItem.tsx

src/redux/chromeReducers.ts

karelhala

Looking good, just remove the filter by source dropdown a it's done in other ticket.

src/components/NotificationsDrawer/DrawerPanelContent.tsx

package.json

Hyperkid123 · 2023-08-31T06:54:47Z

/retest

Hyperkid123 · 2023-08-31T07:07:24Z

/retest

Hyperkid123 · 2023-08-31T07:19:25Z

/retest

src/components/NotificationsDrawer/DrawerPanelContent.tsx

src/components/NotificationsDrawer/NotificationItem.tsx

…ter options.

…ockups now.

arivepr · 2023-09-05T13:36:48Z

Unsure if I did something to cause that new failing test, seems to complaint about missing dockerfile 😕

src/components/NotificationsDrawer/DrawerPanelContent.tsx

src/components/NotificationsDrawer/NotificationItem.tsx

src/redux/index.ts

src/components/NotificationsDrawer/DrawerPanelContent.tsx

Hyperkid123 · 2023-09-06T14:16:02Z

src/components/NotificationsDrawer/DrawerPanelContent.tsx

+        <DropdownList>
+          {filterConfig.map((source) => (
+            <DropdownItem key={source.value} onClick={() => onFilterSelect(source.value)} isDisabled={notifications.length === 0}>
+              <Checkbox isChecked={activeFilters.includes(source.value)} id={source.value} className="pf-v5-u-mr-sm" />


The checkbox is partially uncontrolled which prints an error in a console.

Warning: A component is changing an uncontrolled input to be controlled. This is likely caused by the value changing from undefined to a defined value, which should not happen. Decide between using a controlled or uncontrolled input element for the lifetime of the component. More info: https://reactjs.org/link/controlled-components

There is a build in PF way of creating menu item checkboxes: https://www.patternfly.org/components/menus/menu

DropdownItem shares props with MenuItems. It's just a wrapper around it.

Thanks, I misunderstood what you meant with the menu wrapper before 👍

arivepr requested review from Hyperkid123, ryelo and karelhala as code owners July 17, 2023 07:03

arivepr force-pushed the notification-drawer-items branch from 3ca4569 to 5aaf9e6 Compare July 26, 2023 18:01

arivepr changed the title ~~Notification Drawer Update - DRAFT~~ Notification Drawer Update Jul 26, 2023

karelhala reviewed Aug 3, 2023

View reviewed changes

arivepr force-pushed the notification-drawer-items branch from 2a44e1f to e09c99b Compare August 10, 2023 12:03

arivepr requested a review from florkbr as a code owner August 10, 2023 12:03

arivepr force-pushed the notification-drawer-items branch 2 times, most recently from ffb9320 to c4e188e Compare August 17, 2023 12:04

ryelo requested a review from karelhala August 17, 2023 12:15

ryelo reviewed Aug 17, 2023

View reviewed changes

karelhala reviewed Aug 21, 2023

View reviewed changes

arivepr force-pushed the notification-drawer-items branch from 2b335d3 to e971de9 Compare August 22, 2023 18:04

arivepr force-pushed the notification-drawer-items branch from 2957739 to 9f63bc5 Compare August 23, 2023 19:46

karelhala reviewed Aug 28, 2023

View reviewed changes

src/components/NotificationsDrawer/DrawerPanelContent.tsx Outdated Show resolved Hide resolved

src/components/NotificationsDrawer/DrawerPanelContent.tsx Outdated Show resolved Hide resolved

arivepr force-pushed the notification-drawer-items branch from 9f63bc5 to 5fa9ff3 Compare August 29, 2023 21:36

karelhala reviewed Aug 30, 2023

View reviewed changes

src/components/NotificationsDrawer/NotificationItem.tsx Show resolved Hide resolved

src/redux/chromeReducers.ts Outdated Show resolved Hide resolved

karelhala reviewed Aug 31, 2023

View reviewed changes

package.json Outdated Show resolved Hide resolved

karelhala reviewed Aug 31, 2023

View reviewed changes

src/components/NotificationsDrawer/DrawerPanelContent.tsx Outdated Show resolved Hide resolved

karelhala reviewed Aug 31, 2023

View reviewed changes

src/components/NotificationsDrawer/DrawerPanelContent.tsx Outdated Show resolved Hide resolved

karelhala reviewed Sep 4, 2023

View reviewed changes

src/components/NotificationsDrawer/NotificationItem.tsx Outdated Show resolved Hide resolved

arivepr force-pushed the notification-drawer-items branch from d7c831f to cd35821 Compare September 5, 2023 13:09

arivepr added 15 commits September 5, 2023 09:16

First pass for notification drawers, dropdowns and filters.

5b08115

Fixed issue with notifications items sharing state, and repeating fil…

c4caa32

…ter options.

Connected redux management for notifications.

04fd7ee

Adding source filtering functionality.

6c900fd

Refactoring reducers/actions and other parts of code.

a0e0e01

Switching to dispatchion action creator

aa4c288

Fixed marking as read issue.

dda00c8

Refactoring of notification filter menu and functionality. Matching m…

e3bfecf

…ockups now.

Fixing PF5 related issues after rebasing

a4b8c21

Switching to lodash sorting, fixed visual bug

e140689

Switching to FEC

354f354

Fixing dropdowns, refactoring code, switching to dynamic imports

f30a632

Further code refactoring, and adding navigation links.

bcaa731

Changing data loading, adding dropdown options and refactoring

9362cd6

Updating utils after refactoring.

58dcec0

arivepr force-pushed the notification-drawer-items branch from cd35821 to 58dcec0 Compare September 5, 2023 13:19

karelhala reviewed Sep 5, 2023

View reviewed changes

src/components/NotificationsDrawer/DrawerPanelContent.tsx Outdated Show resolved Hide resolved

Closing drawer on navigation, and fixed small filtering bug.

d40694e

arivepr force-pushed the notification-drawer-items branch from a75ba08 to d40694e Compare September 5, 2023 18:24

Adding notifications initial state to redux.

7937934

karelhala reviewed Sep 6, 2023

View reviewed changes

src/components/NotificationsDrawer/NotificationItem.tsx Outdated Show resolved Hide resolved

src/redux/index.ts Outdated Show resolved Hide resolved

arivepr added 2 commits September 6, 2023 07:44

Adding dropdown wrapper and adjusting bell icon logic.

41f50d5

Readjusting code line.

351ba96

Hyperkid123 reviewed Sep 6, 2023

View reviewed changes

arivepr and others added 2 commits September 6, 2023 14:00

Eliminating console errors.

e9d3834

Merge branch 'master' into notification-drawer-items

668c93f

karelhala merged commit 392b9dd into RedHatInsights:master Sep 7, 2023
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Notification Drawer Update #2571

Notification Drawer Update #2571

arivepr commented Jul 17, 2023 •

edited

Loading

arivepr commented Jul 26, 2023 •

edited

Loading

karelhala left a comment

ryelo Aug 17, 2023

arivepr Aug 17, 2023

arivepr commented Aug 18, 2023

karelhala left a comment

codecov-commenter commented Aug 22, 2023 •

edited

Loading

arivepr commented Aug 22, 2023

karelhala left a comment

arivepr commented Aug 29, 2023 •

edited

Loading

karelhala left a comment

karelhala left a comment

Hyperkid123 commented Aug 31, 2023

Hyperkid123 commented Aug 31, 2023

Hyperkid123 commented Aug 31, 2023

arivepr commented Sep 5, 2023

Hyperkid123 Sep 6, 2023

arivepr Sep 6, 2023

Notification Drawer Update #2571

Notification Drawer Update #2571

Conversation

arivepr commented Jul 17, 2023 • edited Loading

arivepr commented Jul 26, 2023 • edited Loading

karelhala left a comment

Choose a reason for hiding this comment

ryelo Aug 17, 2023

Choose a reason for hiding this comment

arivepr Aug 17, 2023

Choose a reason for hiding this comment

arivepr commented Aug 18, 2023

karelhala left a comment

Choose a reason for hiding this comment

codecov-commenter commented Aug 22, 2023 • edited Loading

Codecov Report

arivepr commented Aug 22, 2023

karelhala left a comment

Choose a reason for hiding this comment

arivepr commented Aug 29, 2023 • edited Loading

karelhala left a comment

Choose a reason for hiding this comment

karelhala left a comment

Choose a reason for hiding this comment

Hyperkid123 commented Aug 31, 2023

Hyperkid123 commented Aug 31, 2023

Hyperkid123 commented Aug 31, 2023

arivepr commented Sep 5, 2023

Hyperkid123 Sep 6, 2023

Choose a reason for hiding this comment

arivepr Sep 6, 2023

Choose a reason for hiding this comment

arivepr commented Jul 17, 2023 •

edited

Loading

arivepr commented Jul 26, 2023 •

edited

Loading

codecov-commenter commented Aug 22, 2023 •

edited

Loading

arivepr commented Aug 29, 2023 •

edited

Loading