CONSOLE-5361: Add toast overflow and notification drawer integration#16636
Open
galkremer1 wants to merge 1 commit into
Open
CONSOLE-5361: Add toast overflow and notification drawer integration#16636galkremer1 wants to merge 1 commit into
galkremer1 wants to merge 1 commit into
Conversation
openshift-ci-robot
added
the
jira/valid-reference
Contributor
|
@galkremer1: This pull request references CONSOLE-5361 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci
Bot
added
the
needs-rebase
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughExtends ChangesToast Notification History & Drawer Integration
Sequence Diagram(s)sequenceDiagram
participant Plugin as Plugin / App Code
participant ConnectedToastProvider
participant ToastProvider
participant History as Notification History
participant Drawer as NotificationDrawer
participant Masthead as Masthead Badge
Plugin->>ToastProvider: addToast({ content, drawerGroup, persistInDrawer })
ToastProvider->>ToastProvider: normalizeToastOptions, assign id/timestamp
alt persistInDrawer !== false
ToastProvider->>History: push ToastNotification
end
alt isNotificationDrawerExpanded
ToastProvider->>ToastProvider: skip visible rendering
else
ToastProvider->>ToastProvider: add to visible toasts
ToastProvider->>ToastProvider: compute getVisibleToasts(maxDisplayed)
end
ToastProvider-->>Plugin: render AlertGroup ± overflow link
Masthead->>History: useNotificationHistory
Masthead->>Masthead: unreadCount, hasUnreadDangerNotifications
Masthead->>Masthead: notificationCount = alertCount + toastUnreadCount
Masthead->>Masthead: getNotificationsVariant → badge variant
Drawer->>History: useNotificationHistory
Drawer->>Drawer: groupToastNotifications
Drawer->>Drawer: render ToastNotificationDrawerItems per group
Drawer->>Drawer: header: unread count + "Mark all read"/"Clear all"
Plugin->>ConnectedToastProvider: drawerToggle
ConnectedToastProvider->>ConnectedToastProvider: notificationDrawerToggleExpanded
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
Bot
added
component/sdk
galkremer1
force-pushed
the
CONSOLE-5361-add-toast-overvflow-drawer-integration
branch
from
ffd43a2 to
8779e3c
Compare
openshift-ci
Bot
removed
the
needs-rebase
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: galkremer1 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
frontend/packages/console-shared/src/components/toast/NotificationHistoryContext.tsx (1)
4-6: Consider using a null default with a clear error check in the hook.
createContext({} as NotificationHistoryContextValues)allows the hook to silently return an empty object if called outside a provider, deferring failures to undefined callback access. While all current consumers are properly nested underToastProvider(viaConnectedToastProviderin app.tsx), change the default tonulland add a validation check inuseNotificationHistoryto fail fast with a helpful error message if the hook is ever called outside the provider context.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/packages/console-shared/src/components/toast/NotificationHistoryContext.tsx` around lines 4 - 6, The default value for NotificationHistoryContext is currently an empty object cast as the type, which silently returns an empty object if the useNotificationHistory hook is called outside the provider context, deferring failures until undefined callback access. Change the default value in createContext from {} to null, then add a validation check at the beginning of the useNotificationHistory hook that verifies the context value is not null and throws a clear error message if the hook is called outside the NotificationHistoryContext provider.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/packages/console-dynamic-plugin-sdk/release-notes/4.23.md`:
- Line 63: The markdown heading "#### Example: grouping toasts in the
notification drawer" skips a level after the "## Changes to shared modules and
API" header, violating the markdownlint heading-increment rule. Change the
four-hash heading (####) to a three-hash heading (###) to maintain proper
heading hierarchy, going from level 2 to level 3 instead of skipping to level 4.
In `@frontend/packages/console-shared/src/components/toast/toastDisplayUtils.ts`:
- Around line 23-26: The slice operation on getCappedToasts(toasts) in the
visibleCappedIds calculation does not clamp the maxDisplayed parameter before
use, causing negative values to produce unexpected slice behavior (JavaScript
slice returns all-but-last items for negative indices) which is inconsistent
with the overflow logic elsewhere that treats maxDisplayed <= 0 as 0. Clamp
maxDisplayed to a non-negative value using Math.max(0, maxDisplayed) before
passing it to the slice operation to ensure consistent behavior across all uses
of maxDisplayed in this function.
In `@frontend/packages/console-shared/src/components/toast/ToastProvider.tsx`:
- Around line 209-216: The AlertGroup component at line 209 is currently only
gated by the toasts.length condition, allowing toasts to remain visible even
when the notification drawer is expanded. Modify the condition that wraps the
AlertGroup rendering to additionally check whether the notification drawer is
open/expanded, and only render the AlertGroup when both conditions are met:
toasts exist AND the drawer is not expanded. This will hide the toasts when the
drawer is opened, maintaining the intended drawer-suppressed behavior.
In `@frontend/public/components/ToastNotificationDrawerItems.tsx`:
- Around line 1-2: Replace all usages of `React.Ref` with the directly imported
`Ref` type to comply with TypeScript's `allowUmdGlobalAccess=false`
configuration. In frontend/public/components/ToastNotificationDrawerItems.tsx
(lines 1-2), add `Ref` to the import statement from `react`, then at line 65
replace `React.Ref` with `Ref`. In
frontend/public/components/notification-drawer.tsx (line 543), replace the usage
of `React.Ref` with `Ref` which is already imported in that file's imports
section.
In `@frontend/public/locales/en/public.json`:
- Around line 1796-1797: The "View {{count}} more notification(s)_one" key uses
"notification(s)" which creates awkward user-visible copy for the singular form.
Replace "(s)" with nothing in the message for the _one form so it reads "View
{{count}} more notification" (singular only), while keeping the _other form as
"View {{count}} more notifications" (plural).
In `@frontend/public/locales/es/public.json`:
- Line 1130: The Spanish locale file is missing translations for several new
notification-drawer feature keys that were added to English. In the
es/public.json file, add Spanish translations for all the missing
notification-drawer keys including "Notification drawer actions", "Mark all
read", "Notification actions", "Mark as read", "Mark as unread", and any other
overflow or action-related text introduced by this feature. Ensure each new key
has a corresponding Spanish value entry matching the structure of the existing
"Clear all" entry. After adding all translations, run yarn i18n as per the
frontend i18n update guidelines and commit the updated locale file alongside the
code changes.
---
Nitpick comments:
In
`@frontend/packages/console-shared/src/components/toast/NotificationHistoryContext.tsx`:
- Around line 4-6: The default value for NotificationHistoryContext is currently
an empty object cast as the type, which silently returns an empty object if the
useNotificationHistory hook is called outside the provider context, deferring
failures until undefined callback access. Change the default value in
createContext from {} to null, then add a validation check at the beginning of
the useNotificationHistory hook that verifies the context value is not null and
throws a clear error message if the hook is called outside the
NotificationHistoryContext provider.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 9749499f-1680-497d-b6a8-349c9cea4645
📒 Files selected for processing (25)
frontend/packages/console-dynamic-plugin-sdk/CHANGELOG-core.mdfrontend/packages/console-dynamic-plugin-sdk/docs/api.mdfrontend/packages/console-dynamic-plugin-sdk/release-notes/4.23.mdfrontend/packages/console-dynamic-plugin-sdk/src/api/core-api.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/console-types.tsfrontend/packages/console-shared/src/components/toast/NotificationHistoryContext.tsxfrontend/packages/console-shared/src/components/toast/ToastProvider.tsxfrontend/packages/console-shared/src/components/toast/__tests__/ToastProvider.spec.tsxfrontend/packages/console-shared/src/components/toast/__tests__/toastDisplayUtils.spec.tsfrontend/packages/console-shared/src/components/toast/__tests__/toastNotificationUtils.spec.tsfrontend/packages/console-shared/src/components/toast/toastDisplayUtils.tsfrontend/packages/console-shared/src/components/toast/toastNotificationUtils.tsfrontend/packages/console-shared/src/components/toast/types.tsfrontend/packages/console-shared/src/components/toast/useNotificationHistory.tsfrontend/public/components/ToastNotificationDrawerItems.tsxfrontend/public/components/app.tsxfrontend/public/components/masthead/masthead-toolbar.tsxfrontend/public/components/notification-drawer.tsxfrontend/public/components/toast/ConnectedToastProvider.tsxfrontend/public/locales/en/public.jsonfrontend/public/locales/es/public.jsonfrontend/public/locales/fr/public.jsonfrontend/public/locales/ja/public.jsonfrontend/public/locales/ko/public.jsonfrontend/public/locales/zh/public.json
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
frontend/packages/console-shared/src/components/toast/toastDisplayUtils.ts (1)
23-26:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winClamp
maxDisplayedonce and reuse it in both visibility and overflow calculations.Line 25 uses raw
maxDisplayedinslice, but Line 39 treats non-positive values as0. With negative values, visibility and overflow diverge.Proposed fix
export const getVisibleToasts = ( toasts: (ToastOptions & { id: string })[], maxDisplayed: number, ): (ToastOptions & { id: string })[] => { + const limit = Math.max(0, maxDisplayed); const visibleCappedIds = new Set( getCappedToasts(toasts) - .slice(0, maxDisplayed) + .slice(0, limit) .map((toast) => toast.id), ); @@ export const getOverflowCount = ( toasts: (ToastOptions & { id: string })[], maxDisplayed: number, ): number => { + const limit = Math.max(0, maxDisplayed); const cappedLength = getCappedToasts(toasts).length; - return maxDisplayed > 0 ? Math.max(0, cappedLength - maxDisplayed) : 0; + return Math.max(0, cappedLength - limit); };Also applies to: 39-39
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/packages/console-shared/src/components/toast/toastDisplayUtils.ts` around lines 23 - 26, The maxDisplayed parameter is used inconsistently across visibility and overflow calculations: at line 25 in the slice() call within the visibility calculation, it's used as-is, but at line 39 it's treated as having a minimum value of 0. To fix this divergence, clamp maxDisplayed to a non-negative value once at the beginning of the function (e.g., const clampedMaxDisplayed = Math.max(0, maxDisplayed)), then replace the raw maxDisplayed reference in both the slice() call around line 25 and the overflow calculation around line 39 with the clamped variable to ensure consistent behavior regardless of the input value.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@frontend/packages/console-shared/src/components/toast/toastDisplayUtils.ts`:
- Around line 23-26: The maxDisplayed parameter is used inconsistently across
visibility and overflow calculations: at line 25 in the slice() call within the
visibility calculation, it's used as-is, but at line 39 it's treated as having a
minimum value of 0. To fix this divergence, clamp maxDisplayed to a non-negative
value once at the beginning of the function (e.g., const clampedMaxDisplayed =
Math.max(0, maxDisplayed)), then replace the raw maxDisplayed reference in both
the slice() call around line 25 and the overflow calculation around line 39 with
the clamped variable to ensure consistent behavior regardless of the input
value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 0d430aa7-2c0e-4f25-b084-47d5bd257de4
📒 Files selected for processing (14)
frontend/packages/console-dynamic-plugin-sdk/CHANGELOG-core.mdfrontend/packages/console-dynamic-plugin-sdk/docs/api.mdfrontend/packages/console-dynamic-plugin-sdk/release-notes/4.23.mdfrontend/packages/console-dynamic-plugin-sdk/src/api/core-api.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/console-types.tsfrontend/packages/console-shared/src/components/toast/NotificationHistoryContext.tsxfrontend/packages/console-shared/src/components/toast/ToastProvider.tsxfrontend/packages/console-shared/src/components/toast/__tests__/ToastProvider.spec.tsxfrontend/packages/console-shared/src/components/toast/__tests__/toastDisplayUtils.spec.tsfrontend/packages/console-shared/src/components/toast/__tests__/toastNotificationUtils.spec.tsfrontend/packages/console-shared/src/components/toast/toastDisplayUtils.tsfrontend/packages/console-shared/src/components/toast/toastNotificationUtils.tsfrontend/packages/console-shared/src/components/toast/types.tsfrontend/packages/console-shared/src/components/toast/useNotificationHistory.ts
✅ Files skipped from review due to trivial changes (4)
- frontend/packages/console-shared/src/components/toast/useNotificationHistory.ts
- frontend/packages/console-dynamic-plugin-sdk/src/api/core-api.ts
- frontend/packages/console-shared/src/components/toast/NotificationHistoryContext.tsx
- frontend/packages/console-dynamic-plugin-sdk/docs/api.md
🚧 Files skipped from review as they are similar to previous changes (7)
- frontend/packages/console-shared/src/components/toast/types.ts
- frontend/packages/console-shared/src/components/toast/tests/toastNotificationUtils.spec.ts
- frontend/packages/console-dynamic-plugin-sdk/src/extensions/console-types.ts
- frontend/packages/console-shared/src/components/toast/tests/toastDisplayUtils.spec.ts
- frontend/packages/console-shared/src/components/toast/toastNotificationUtils.ts
- frontend/packages/console-shared/src/components/toast/ToastProvider.tsx
- frontend/packages/console-shared/src/components/toast/tests/ToastProvider.spec.tsx
galkremer1
force-pushed
the
CONSOLE-5361-add-toast-overvflow-drawer-integration
branch
from
c17fe1a to
7b0573b
Compare
galkremer1
force-pushed
the
CONSOLE-5361-add-toast-overvflow-drawer-integration
branch
from
7b0573b to
dcf1921
Compare
Contributor
|
@galkremer1: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
openshift-ci
Bot
added
the
needs-rebase
Contributor
|
PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
RFE-9430: Add toast overflow and notification drawer integration
Jira ticket: CONSOLE-5361
Analysis / Root cause:
Toast alerts were shown in an unbounded stack with no persistence beyond the on-screen toast. As plugin usage grew (e.g. long-running upload progress toasts), users could end up with many simultaneous alerts, no way to review missed notifications, and no integration with the existing notification drawer.
Related follow-ups tracked separately:
Solution description:
This PR integrates toast alerts with the notification drawer and caps the number of simultaneously visible toasts (default: 3).
Core behavior
ToastProvidermaintains two states: visible on-screen toasts and notification history for the drawer.AlertGroupshows a "View N more notification(s)" overflow link that clears the visible stack and opens the notification drawer.attention) or other unread toasts (unread).Plugin API (
ToastOptions)drawerGroupskipOverflowfalsepersistInDrawertruefalse, toast is on-screen only (legacy behavior)Constraints:
persistInDrawer: falseimpliesskipOverflow: true(enforced bynormalizeToastOptions) so ephemeral toasts cannot be hidden by overflow with no drawer entry.default) and is displayed as translated Other Alerts. CustomdrawerGroupvalues are shown as-is; plugins are responsible for their own i18n.Architecture
ConnectedToastProvider- wires Redux notification drawer state intoToastProvider.NotificationHistoryContext/useNotificationHistory- exposes drawer history to masthead and notification drawer.toastDisplayUtils- overflow cap and visibility logic.toastNotificationUtils- drawer grouping helpers, default group title translation, andgetNotificationsVariant.ToastNotificationDrawerItems- drawer UI for toast history items (public package).useRefso opening the notification drawer does not reset IDs and overwrite unrelated drawer notifications.SDK / docs
CHANGELOG-core.md,release-notes/4.23.md,docs/api.md, andcore-api.tsJSDoc updated.Usage examples
Default behavior - toast on screen, persisted in drawer under Other Alerts, subject to overflow cap:
Grouped in the notification drawer (custom section title shown as-is):
Always visible on screen, still persisted in drawer (e.g. in-progress upload with Cancel action):
Legacy ephemeral toast - on screen only, not in drawer, never hidden by overflow:
Terminal toast with cleanup when dismissed from drawer or close button:
Demo
These recordings use the KubeVirt plugin as a dynamic-plugin consumer of the Console toast APIs.
1. Default drawer integration
What to look for:
drawerGroupcreates a dedicated drawer section titled Custom Drawer Group (shown as-is; Console does not translate custom group names).Screen.Recording.2026-06-16.at.9.05.17.PM.mov
2. Ephemeral / on-screen only
What to look for:
skipOverflow: truewhenpersistInDrawerisfalse, so the toast is never hidden by the overflow cap (there is no drawer entry to recover it from).Screen.Recording.2026-06-16.at.9.08.57.PM.mov
3. Always-visible + drawer-persisted (upload progress)
What to look for:
skipOverflow: true).persistInDrawer: true).Screen.Recording.2026-06-16.at.9.11.02.PM.mov
Test setup:
yarn devinfrontend/).useToast(e.g. KubeVirt) to exercise plugin toasts withdrawerGroup,skipOverflow, andpersistInDrawer.Test cases:
onCloseinvoked when clearing from drawer.onCloseinvoked for each.drawerGroup: 'Uploads'→ appears under custom drawer section (not under Other Alerts).persistInDrawer: false→ shown on screen only; not in drawer; always visible regardless of cap.skipOverflow: true→ always visible on screen; still in drawer unlesspersistInDrawer: false.attentionvariant.unreadvariant.onCloseinvoked.Browser conformance:
Additional info:
packages/console-shared/src/components/toast/(ToastProvider, toastDisplayUtils, toastNotificationUtils, useToast).en/public.json; other locales include manual Clear all entry - remaining strings via Memsource.onClosecallbacks; consumers with long-lived toasts should useskipOverflow/persistInDrawerappropriately or handle lifecycle viaonClose.Reviewers and assignees
Summary by CodeRabbit
ToastOptionssupport fordrawerGroup,skipOverflow, andpersistInDrawer, plus per-notification drawer actions (mark read/unread, clear).skipOverflow/persistInDrawercombinations.