CONSOLE-5424: Add minimize action for toast notifications#16762
CONSOLE-5424: Add minimize action for toast notifications#16762galkremer1 wants to merge 3 commits into
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
@galkremer1: This pull request references CONSOLE-5424 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. |
|
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:
WalkthroughThe toast SDK and provider now support minimizing drawer-persisted toasts through icon controls, configured actions, and ChangesToast minimization
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Consumer
participant ToastProvider
participant Notifications
Consumer->>ToastProvider: call minimizeToast(id)
ToastProvider->>ToastProvider: remove persisted toast from visible toasts
ToastProvider->>Notifications: mark notification unread
Suggested reviewers: 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
frontend/packages/console-dynamic-plugin-sdk/src/extensions/console-types.ts (1)
637-643: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument that minimization does not invoke
onRemove.
ToastOptionsalready exposesonRemove, but these new descriptions only state thatonCloseis not called. Since minimization removes the toast from the visible list without invoking either lifecycle callback, document theonRemovebehavior here to make the contract unambiguous.Also applies to: 678-696
🤖 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-dynamic-plugin-sdk/src/extensions/console-types.ts` around lines 637 - 643, Update the minimize option documentation in ToastOptions, including the related description around onRemove, to explicitly state that minimization does not invoke onRemove in addition to onClose. Preserve the existing behavior and wording for persistence and dismiss precedence.
🤖 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-shared/src/components/toast/ToastProvider.tsx`:
- Around line 235-297: Differentiate the fallback data-test identifiers for the
minimize icon button and minimize action link in the visibleToasts rendering.
Keep toast.dataTest-derived identifiers unchanged, but replace the shared
default 'toast-minimize-action' value for one control with a distinct identifier
so combined minimize controls remain uniquely selectable.
- Around line 121-134: Update minimizeToast so the notification state update
occurs only when the matching toast exists and has persistInDrawer === true,
matching the existing toast-removal guard; otherwise return without changing
either state.
---
Nitpick comments:
In
`@frontend/packages/console-dynamic-plugin-sdk/src/extensions/console-types.ts`:
- Around line 637-643: Update the minimize option documentation in ToastOptions,
including the related description around onRemove, to explicitly state that
minimization does not invoke onRemove in addition to onClose. Preserve the
existing behavior and wording for persistence and dismiss precedence.
🪄 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: c1dedd4f-3bda-4f39-97d6-c50a87536bd2
📒 Files selected for processing (9)
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/locales/en/console-shared.jsonfrontend/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__/useToast.spec.tsx
0ddcfc8 to
843a70a
Compare
e12dbb0 to
28ef567
Compare
|
/retest-backend |
jseseCCS
left a comment
There was a problem hiding this comment.
looks great! only 3 comments (and 1 is the same comment in 2 different files). conditionally approving in good faith that you'll consider and apply as appropriate. 😺
| ### Returns | ||
|
|
||
| A toast context object with an `addToast` function for adding a toast to the screen and a `removeToast` function for removing a toast from the screen. | ||
| A toast context object with an `addToast` function for adding a toast to the screen, a<br/>`removeToast` function for removing a toast from the screen, and a `minimizeToast` function for<br/>hiding a drawer-persisted toast from the screen while keeping it unread in the notification drawer. |
There was a problem hiding this comment.
Nit: reads a bit rough w/
tags breaking mid-clause, and first two items in list aren't separated by "and" the way a three-item list normally is. suggested rewrite? -->
"A context object with an addToast function for adding a toast to the screen, a removeToast function for removing it from the screen, and a minimizeToast function for hiding a drawer-persisted toast from the screen while keeping it unread in the notification drawer."
...drops the
tags = wrap naturally, unless they're needed for rendering reason?
There was a problem hiding this comment.
Updated. The tags <br/> for adding new lines are auto generated.
This text is coming from frontend/packages/console-dynamic-plugin-sdk/src/api/core-api.ts file you commented on down below.
|
|
||
| - `persistInDrawer` (default `false`): persists the toast in the drawer with read/unread state. Defaults | ||
| `skipOverflow` to `false` so it participates in the visible cap (default 3) and overflow link. | ||
| - `skipOverflow` (default `true`): excludes the toast from the visible cap and overflow link. Combine |
There was a problem hiding this comment.
67-69
a little confusing. skipOverflow listed w/default of true, but persistInDrawer bullet says it defaults to false in that case. Could skipOverflow line clarify that it defaults to false when persistInDrawer is true, and true otherwise? something like:
"skipOverflow: when persistInDrawer is true, defaults to false; otherwise defaults to true."
?
that would help user skimming one bullet at a time.
There was a problem hiding this comment.
I agree - it is confusing
Updating to
- `persistInDrawer` (default `false`): persists the toast in the drawer with read/unread state.
- `skipOverflow`: excludes the toast from the visible cap (default 3) and overflow link. Defaults to
`true`, except when `persistInDrawer: true`, where it defaults to `false` so drawer-persisted toasts
count toward the cap. Set `skipOverflow: true` explicitly to keep a drawer-persisted toast always
visible.
| * @returns A toast context object with an `addToast` function for adding a toast to the screen and a `removeToast` function for removing a toast from the screen. | ||
| * @returns A toast context object with an `addToast` function for adding a toast to the screen, a | ||
| * `removeToast` function for removing a toast from the screen, and a `minimizeToast` function for | ||
| * hiding a drawer-persisted toast from the screen while keeping it unread in the notification drawer. |
|
/label docs-approved |
28ef567 to
18def4f
Compare
18def4f to
16fbba0
Compare
| - Add optional `onSubmit` parameter to `useLabelsModal` hook for customizing label submission behavior ([CONSOLE-5356], [#16560]) | ||
| - Update `@patternfly/react-topology` peer dependency semver range to `~6.6.0` ([OCPBUGS-86587], [#16750]) | ||
| - Update `react-router` peer dependency semver range to `~7.18.1` ([CONSOLE-5415], [#16726]) | ||
| - Add minimize action for toast notifications via new `ToastOptions.minimizable`, `ToastOptions.actions[].minimize`, and `ToastContextValues.minimizeToast` ([CONSOLE-5424], [#16762]) |
There was a problem hiding this comment.
make a new section ## 4.23.0-prerelease.5
There was a problem hiding this comment.
Yes sorry I missed it in the rebase.
|
|
||
| [CONSOLE-5361]: https://issues.redhat.com/browse/CONSOLE-5361 | ||
| [CONSOLE-5424]: https://issues.redhat.com/browse/CONSOLE-5424 |
There was a problem hiding this comment.
please no jira links in release notes. jira links are for the changelog
| `useToast` toasts can now persist in the notification drawer, cap how many show on-screen, and be | ||
| minimized back to the drawer. Existing consumers are unaffected — by default, toasts stay on-screen | ||
| only, with no cap. ([CONSOLE-5361], [CONSOLE-5424]) | ||
|
|
||
| `ToastOptions` includes: | ||
|
|
||
| - `persistInDrawer` (default `false`): persists the toast in the drawer with read/unread state. | ||
| - `skipOverflow`: excludes the toast from the visible cap (default 3) and overflow link. Defaults to | ||
| `true`, except when `persistInDrawer: true`, where it defaults to `false` so drawer-persisted toasts | ||
| count toward the cap. Set `skipOverflow: true` explicitly to keep a drawer-persisted toast always | ||
| visible. | ||
| - `drawerGroup`: groups the toast into a named drawer section. Omitted toasts use the built-in group, | ||
| displayed as translated "Other Alerts". Custom values are shown as-is; plugins are responsible for | ||
| translating their own group names. | ||
| - `minimizable` (default `false`): for toasts that aren't `dismissible`, shows a Minus icon button in the | ||
| close-button slot that minimizes the toast — hides it on-screen while keeping it unread in the drawer, | ||
| without invoking `onClose`. No-op unless `persistInDrawer: true`. | ||
|
|
||
| An action with `minimize: true` in `actions` renders the same behavior as a text link instead, at | ||
| whatever position you place it in the array — useful for `dismissible` toasts (the icon slot is taken by | ||
| the close button) or to control action order. `label`/`callback` are required, same as any other action. | ||
| `minimizable` and an `actions[].minimize` entry are independent and can be combined. `useToast` also | ||
| returns `minimizeToast(id)` for minimizing programmatically. |
There was a problem hiding this comment.
please tell the LLM to write more terse, no em-dashes, more human like, etc.
|
@coderabbitai run the microcopy-review skill on this PR |
|
This command requires write access to the repository. Only users with write or admin permissions can trigger CodeRabbit to commit or create pull requests. |
| `useToast` toasts can now persist in the notification drawer, cap how many show on-screen, and be | ||
| minimized back to the drawer. Existing consumers are unaffected: by default, toasts stay on-screen only, | ||
| with no cap. |
There was a problem hiding this comment.
useToast was also exposed in this release, right? "can now" doesn't make sense in this context then and this section really should be talking about useToast as a whole
There was a problem hiding this comment.
Yes this is true. I am rewriting this section now.
e033766 to
b649791
Compare
| place it anywhere among your other actions, for example on `dismissible` toasts. `useToast` also returns | ||
| `minimizeToast(id)` for minimizing programmatically. | ||
|
|
||
| ### Example: Persist, group, and minimize a toast |
There was a problem hiding this comment.
per heading semantics the examples should be h4
b649791 to
8da5b15
Compare
| ``` | ||
|
|
||
| ### Example: persisting and grouping toasts in the notification drawer | ||
| ### Example: Order a minimize action for a dismissible toast |
4af2f98 to
e4c9353
Compare
e4c9353 to
4c86298
Compare
| with an overflow link to the notification drawer. Existing toast consumers are unaffected: by default, | ||
| toasts remain on-screen only. | ||
|
|
||
| `ToastOptions` supports optional properties for this control: |
There was a problem hiding this comment.
@jseseCCS can you please review this section again?
| ### Returns | ||
|
|
||
| A toast context object with an `addToast` function for adding a toast to the screen and a `removeToast` function for removing a toast from the screen. | ||
| A context object with an addToast function for adding a toast to the screen, a<br/>removeToast function for removing it from the screen, and a minimizeToast function for<br/>hiding a drawer-persisted toast from the screen while keeping it unread in the notification drawer. |
There was a problem hiding this comment.
the breaks have to stay @galkremer1 ?
There was a problem hiding this comment.
Yes, they are auto-generated by the content of frontend/packages/console-dynamic-plugin-sdk/src/api/core-api.ts
|
/retest-backend |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: galkremer1, jseseCCS, logonoff The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Scheduling tests matching the |
|
/test e2e-gcp-console |
|
/retest |
|
/test e2e-playwright |
2 similar comments
|
/test e2e-playwright |
|
/test e2e-playwright |
|
@galkremer1: The following test 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. |
|
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. |
RFE-9433: Add minimize action for toast notifications
Jira ticket: CONSOLE-5424
Analysis / Root cause:
CONSOLE-5361 added drawer persistence and an overflow cap for toasts, but gave users no direct way to hide a single toast from the on-screen alert group without either dismissing it (which cancels/loses the underlying operation via
onClose) or waiting for the overflow cap to push it off-screen. Long-running, non-dismissible toasts (e.g. upload progress) had no "get this out of my way, but keep it running and keep the history" action. This follow-up was tracked separately as RFE-9433 in the CONSOLE-5361 PR description.Solution description:
This PR adds a Minimize action for toast notifications. Minimizing hides a toast from the on-screen
AlertGroupwhile keeping its notification-drawer entry (marked unread), without invokingonClose/onRemoveand without canceling whatever operation the toast represents.Consumers can surface Minimize in two independent ways, and can combine both on the same toast:
minimizable: true): for toasts that aren'tdismissible, Console renders a Minus icon button in the close-button slot, since there's no close button to collide with there. This is the only control for the icon button — if you don't want it, just leaveminimizableunset.minimize: trueon an entry inactions): renders as a text action link at whatever position the consumer places it in theactionsarray, so they control the order relative to their own actions.labelandcallbackare required, same as any other action — use a no-op callback if you don't need one. This is the only option fordismissibletoasts, since the close button already occupies the icon slot.Both mechanisms only take effect when the toast is persisted in the drawer (
persistInDrawer: true) — there is no drawer entry to fall back to otherwise. Existing toast consumers are unaffected —minimizableandactions[].minimizeboth default to off, preserving pre-PR behavior.Core behavior
ToastProvider.minimizeToast(id)hides the toast from the visible on-screen stack and marks its drawer notification unread, without invokingonClose/onRemoveand without removing the drawer history entry.persistInDrawer: true— there is no drawer entry to recover it from.useToast().minimizeToast, alongside the existingaddToast/removeToast.Plugin API (
ToastOptions)minimizablefalsetrueanddismissibleisfalse, shows a Minus icon button in the close button's slot that minimizes the toast. This is the only control for the icon button — omit it to use just the action-link versionactions[].minimizefalsetrueon an action entry, clicking it minimizes the toast instead of the normal callback/dismiss flow. Renders as a text link at that action's exact position.label/callbackare still required, same as any other actionConstraints:
persistInDrawer: true— there's no drawer entry to fall back to otherwise.minimizableandactions[].minimizeare independent; both render if both are configured on the same toast, and there's no separate "disable" flag — leavingminimizableunset is how you get only the action-link version.actions[].minimizetakes precedence overdismisswhen both are set on the same action; the action'scallbackis still invoked in addition to minimizing (use a no-op if you don't need one).Architecture
ToastProvider- newminimizeToastcallback;actionClose/actionLinksrendering updated to support the icon button and/or action-link variants.console-types.ts(SDK) -ToastOptions.minimizable,ToastOptions.actions[].minimize,ToastContextValues.minimizeToast.SDK / docs
CHANGELOG-core.md,release-notes/4.23.md,docs/api.md, andcore-api.tsJSDoc updated.Usage examples
Automatic icon button — non-dismissible, always-visible toast (e.g. upload progress):
Explicit action, ordered relative to other actions, on a dismissible toast:
Explicit action with a custom label:
Combining the icon button and an explicit action — both controls render, since the two are independent:
Demo
1. Automatic icon button (no
actions)What to look for:
dismissible: false).onClose/onRemoveare not invoked — any underlying operation (e.g. upload) is unaffected.Demo1.mov
2. Explicit
minimizeaction, ordered among other actionsWhat to look for:
actionsarray (here, after "Retry"), with whateverlabelwas provided.onClose.Demo2.mov
3. Combining the icon button and an explicit action
What to look for:
minimizableandactions[].minimizeare independent controls: when both are configured, both render (the icon button and the action link) — there's no implicit suppression either way.minimizableunset.Demo3.mov
Test setup:
yarn devinfrontend/).useToast(e.g. KubeVirt) to exercise plugin toasts withminimizableandactions[].minimize.Test cases:
minimizable: true, persistInDrawer: true, dismissible: false→ Minus icon button renders in the close-button slot; clicking it hides the toast, keeps it unread in the drawer, and does not invokeonClose.dismissible: true→ the Minus icon is never shown (close button occupies that slot); add an action withlabel/minimize: true/callbacktoactionsto get a text link instead.actions: [{ label: 'Hide for now', minimize: true, callback: () => {} }]→ link renders with that label; clicking minimizes (hides, keeps unread, noonClose).minimize: trueentry placed in the middle of the array → the Minimize link renders at that exact position relative to the other actions.minimize: trueaction that also has a non-no-opcallback→ clicking invokes both the minimize behavior and the callback.minimizable: trueand an explicitactions[].minimizeaction → both the icon button and the action link render (the two are independent, neither suppresses the other).actions[].minimizeaction and nominimizable→ only the action link renders; no icon button appears.actions[].minimizeaction but nopersistInDrawer→ the link still renders, but clicking it has no effect (toast stays visible; there is no drawer entry).useToast().minimizeToast(id)directly → same behavior as clicking the UI control.Browser conformance:
Additional info:
packages/console-shared/src/components/toast/(ToastProvider, toastDisplayUtils, toastNotificationUtils, useToast).console-shared.json(Minimize alert: {{title}}, for the icon button's aria-label); other locales via Memsource.addToastcallers (Console core and dynamic plugins) are unaffected.minimizableandactions[].minimizeboth default to off/false, matching pre-PR behavior.Reviewers and assignees
Summary by CodeRabbit