refactor: M3-7034 - Event Messages Part 2 #10550

abailly-akamai · 2024-06-05T21:07:07Z

Description 📝

Part 2 of the Event Messages refactor. This PR addresses primarily:

Progress events
Code cleanup
UI updates

The aim of the PR is to deprecate eventMessageGenerator_CMR. The way it was always fetching Linode, Linode Types and Region data no matter what the event in order to hook some events and modify their message. With the new Event Messages factories we can now just build custom events as components and use them directly in factories actions so fetching is tightly to the action status.

Changes 🔄

Improve and optimize progress event logic
Implement v2 versions of hooks and components (in order for the feature to remain behind a feature flag)
Implement CDS guidelined (UX approved ) UI changes
- notification pill color
- if more than 9 unread notifications, show 9+
- new progress pill icon
- unseen notification border
- relative time, time remaining copy
Close Notification Center when clicking on "View all events" since we land on the event page (UX approved as well)

💡 See the self review comments for more details

🗒️: the time_remaining value is inconsistently coming back from the API and being looked at. It should appear for a lot more progress events (ex, resize, migration) but is apparently currently broken. This is looked at in ARB-5127.

🧪: This PR's unit test coverage is pretty basic since it will be followed by a full e2e suite dedicated to events

Preview 📷

Events Page

Notification Center

Event Bell Notification

How to test 🧪

Verification steps

(How to verify changes)

Checkout PR and make sure the eventMessagesV2 is turned on
Check the of both the event page and the notification for a variety of progress events (boot, reboot, resize, migrate, clone etc)

As an Author I have considered 🤔

Check all that apply

abailly-akamai · 2024-06-10T21:58:09Z

packages/manager/src/assets/icons/account.svg

Tiny change to the icon so it does not appear squished with the proper 1x1 ratio

abailly-akamai · 2024-06-10T21:59:09Z

packages/manager/src/features/Events/EventRowV2.tsx

+            sx={{ mb: 1, mt: 0.5 }}
+            value={event.percent_complete ?? 0}
+          />
+        )}


The progress bar was missing from the events page the whole - it's back in for consistency.

abailly-akamai · 2024-06-10T21:59:50Z

packages/manager/src/features/Events/EventRowV2.tsx

+          <Box alignItems="center" display="flex" gap={1}>
+            <StyledGravatar username={username ?? ''} />
+            {username ?? 'Linode'}
+          </Box>


Avatar is now alongside the username in the same cell

abailly-akamai · 2024-06-10T22:03:19Z

packages/manager/src/features/Events/EventsLanding.tsx

@@ -33,7 +32,7 @@ export const EventsLanding = (props: Props) => {
  const { emptyMessage, entityId } = props;
  const flags = useFlags();

-  const filter: Filter = { action: { '+neq': 'profile_update' } };
+  const filter = EVENT_POLLING_FILTER;


this is so we not filter events API side rather than on the client. This is now consistent wherever events are fetched via useEventsInfiniteQuery.

abailly-akamai · 2024-06-10T22:06:38Z

packages/manager/src/features/Events/constants.ts

+  'image_upload',
+  'volume_migrate',
+  'database_resize',
+];


This replaces the switch at https://github.com/linode/manager/blob/develop/packages/manager/src/features/Events/eventMessageGenerator_CMR.tsx#L25-L91 since this PR is deprecating the legacy eventGenerator handler which was over-fetching and was hard to maintain.

We can now override messages at the factory level if we need to customize the message via additional queries. See events/factories/linode.tsx right below for more details.

abailly-akamai · 2024-06-10T22:10:42Z

packages/manager/src/features/Events/factories/linode.tsx

+      .
+    </>
+  );
+};


As mentioned, if we need to customize a message we can just declare a new component that can deal with fetching new data to compose the message. This way this is only called if the action/status happen in the UI. This is the biggest improvement in this PR. This cam be tested by triggering a Linode resize.

abailly-akamai · 2024-06-10T22:11:50Z

packages/manager/src/features/Events/utils.tsx

+    progressEventDisplay,
+    showProgress,
+  };
+};


Consolidating functionality into utils to make things cleaner and reusable.

abailly-akamai · 2024-06-11T00:16:09Z

packages/manager/src/features/NotificationCenter/NotificationData/RenderEvent.tsx

-      ) : null
-    );
-  }
-


Moved the v2 conditional rendering up the tree

abailly-akamai · 2024-06-11T00:19:09Z

packages/manager/src/features/TopMenu/NotificationMenu/NotificationMenuV2.tsx

This looks like a big new file but it's pretty much a copy of NotificationMenu with the exception of using the v2 hook and some styles changes. I did not really clean up this file or try to modernize it as to avoid functional regressions and keep the scope moderate.

github-actions · 2024-06-11T01:54:42Z

Coverage Report: ✅
Base Coverage: 82.75%
Current Coverage: 82.8%

hkhalil-akamai

Amazing work and improvements to the notification menu, events rendering and small UI tweaks like the in-progress bell indicator. Left some feedback and one curious bug but otherwise this looks great!

packages/manager/src/features/TopMenu/NotificationMenu/NotificationMenuV2.tsx

hkhalil-akamai · 2024-06-11T22:00:50Z

packages/manager/src/features/TopMenu/NotificationMenu/NotificationMenuV2.tsx

+          )}
+        </IconButton>
+      </TopMenuTooltip>
+      <Popover


There is a slight pre-existing flicker caused by the scroll bar disappearing when opening the notifications menu. It's not critical but it is annoying, maybe we can tackle it as part of this refactor?

Screen.Recording.2024-06-11.at.5.58.18.PM.mov

So i am not going to address this one in this PR cause it's a can of worms since it relates to both MUI's global handling of popovers the create menu is doing the same thing for example) and a user's system settings. What happens here is due to the overflow and padding set on the body by MUI). Not that this shouldn't be looked at, but out of scope for this ticket (will make a follow up one).

As an aside, you can use "When Scrolling" in your system settings cause the experience is better and this problem will go away.

hkhalil-akamai · 2024-06-11T22:02:50Z

packages/manager/src/features/TopMenu/NotificationMenu/NotificationMenuV2.tsx

+    if (prevOpen && !notificationContext.menuOpen) {
+      // Dismiss seen notifications after the menu has closed.
+      if (eventNotifications.length > 0) {
+        markEventsAsSeen(eventNotifications[0].eventId);


While testing I did notice a bug with some older events not being marked as seen when opening/closing the menu but I can't replicate it anymore and this code is identical to what we already have so I'm not sure why it would be happening. Still, I will keep an eye on the unread count.

Wait I'm seeing it again but I have no idea why... maybe this is an API issue?

Screen.Recording.2024-06-11.at.6.04.01.PM.mov

I have noticed that as well (seldomly) and was able to reproduce in production when /seen request don't seem to resolve the next event request. Strangely, triggering a new event and new notifications seem to "unblock" the faulty behavior and clearing old events. I'll keep an eye on it and perhaps open an ARB ticket to look at this if this happens a lot more

bnussman-akamai

PR is looking good so far 😎 will revisit tomorrow

packages/manager/src/features/Events/constants.ts

packages/manager/src/features/Events/factories/linode.tsx

packages/manager/src/features/NotificationCenter/NotificationData/RenderEventV2.tsx

bnussman-akamai · 2024-06-11T22:26:25Z

packages/manager/src/features/NotificationCenter/EventsV2.tsx

This is more of a rant about previous code, not about your great work here 😅 :

I feel like there is way more code than necessary needed to render the NotificationMenu in general. There are so many layers to this Menu like NotificationSection, useEventNotifications, FormattedEventForDisplay, and so on. I know most of this is carry over/copy paste from the old system, but I hope we can trim things down and un-abstact some in the future.

I might be mistaken and all of those abstractions are what we want, but I can't help but want to try to make it less convoluted

Agreed! once I clean up all the old files I will see a bit more clearly and consolidate this. This is on my list

jaalah-akamai

Great job @abailly-akamai - Love the new UI

docs/development-guide/15-api-events.md

packages/manager/src/features/Events/utils.tsx

packages/manager/src/features/NotificationCenter/NotificationData/RenderEventV2.tsx

packages/manager/src/features/TopMenu/NotificationMenu/NotificationMenuV2.tsx

abailly-akamai added the Tech Debt label Jun 5, 2024

abailly-akamai self-assigned this Jun 5, 2024

abailly-akamai force-pushed the M3-7034 branch 2 times, most recently from fd6731e to a7a9e61 Compare June 7, 2024 16:29

abailly-akamai commented Jun 11, 2024

View reviewed changes

abailly-akamai marked this pull request as ready for review June 11, 2024 01:27

abailly-akamai requested a review from a team as a code owner June 11, 2024 01:27

abailly-akamai requested review from hkhalil-akamai, jaalah-akamai and bnussman-akamai and removed request for a team June 11, 2024 01:27

hkhalil-akamai approved these changes Jun 11, 2024

View reviewed changes

bnussman-akamai reviewed Jun 11, 2024

View reviewed changes

abailly-akamai force-pushed the M3-7034 branch from a64bc01 to 7fcb918 Compare June 12, 2024 13:09

bnussman-akamai self-requested a review June 12, 2024 22:36

jaalah-akamai approved these changes Jun 13, 2024

View reviewed changes

bnussman-akamai approved these changes Jun 13, 2024

View reviewed changes

hkhalil-akamai approved these changes Jun 13, 2024

View reviewed changes

abailly-akamai force-pushed the M3-7034 branch from 08fc7e6 to 93faea2 Compare June 13, 2024 18:25

abailly-akamai added 10 commits June 13, 2024 16:44

Initial commit: trying things

00ad734

save wip

c87d1ec

save progres

c1874e1

save work

8982148

save work

e650e96

custom message

da7bce0

more custom message and formatting

719b89a

UI touchups

48b39e2

UI touchups 2

cac7a7b

cleanup

2509929

abailly-akamai added 6 commits June 13, 2024 16:45

adding coverage

3a333eb

UI updates

b5dafb0

Documentation

2f9506b

Added changeset: Event Messages Refactor: progress events

4962f78

feedback @hkhalil-akamai & @bnussman-akamai

548176a

feedback @jaalah-akamai

7ef2173

abailly-akamai force-pushed the M3-7034 branch from b4bdc87 to 7ef2173 Compare June 13, 2024 20:45

abailly-akamai merged commit 6ab8240 into linode:develop Jun 14, 2024
18 checks passed

This was referenced Jun 21, 2024

Release v1.122.0 - release → staging #10601

Merged

Release v1.122.0 - staging → master #10608

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: M3-7034 - Event Messages Part 2 #10550

refactor: M3-7034 - Event Messages Part 2 #10550

abailly-akamai commented Jun 5, 2024 •

edited

Loading

abailly-akamai Jun 10, 2024

abailly-akamai Jun 10, 2024

abailly-akamai Jun 10, 2024

abailly-akamai Jun 10, 2024

abailly-akamai Jun 10, 2024

abailly-akamai Jun 10, 2024

abailly-akamai Jun 10, 2024

abailly-akamai Jun 11, 2024

abailly-akamai Jun 11, 2024

github-actions bot commented Jun 11, 2024 •

edited

Loading

hkhalil-akamai left a comment

hkhalil-akamai Jun 11, 2024

abailly-akamai Jun 12, 2024

hkhalil-akamai Jun 11, 2024

hkhalil-akamai Jun 11, 2024

abailly-akamai Jun 12, 2024

bnussman-akamai left a comment

bnussman-akamai Jun 11, 2024

abailly-akamai Jun 12, 2024

jaalah-akamai left a comment

refactor: M3-7034 - Event Messages Part 2 #10550

refactor: M3-7034 - Event Messages Part 2 #10550

Conversation

abailly-akamai commented Jun 5, 2024 • edited Loading

Description 📝

Changes 🔄

Preview 📷

How to test 🧪

Verification steps

As an Author I have considered 🤔

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jun 11, 2024 • edited Loading

hkhalil-akamai left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bnussman-akamai left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaalah-akamai left a comment

Choose a reason for hiding this comment

abailly-akamai commented Jun 5, 2024 •

edited

Loading

github-actions bot commented Jun 11, 2024 •

edited

Loading