[Notifications] Move notification settings into the Notifications widget - #4084
[Notifications] Move notification settings into the Notifications widget#4084idaiv wants to merge 4 commits into
Conversation
Notification settings used to be a second top-level widget, reachable only from a gear icon in the user menu. It is now a section of the Notifications widget itself, alongside the inbox. - Add a tabbed widget shell (Inbox / Settings). Each section renders its own ContentLayout, because their toolbars have nothing in common (the inbox pages and clears, settings saves and discards) and the filter sidebar belongs to the inbox alone. Panes stay mounted so switching sections cannot drop an unsaved settings draft. - Drop the standalone `notification-settings` widget and its NOTIFICATION_SETTINGS config. Persisted layouts referencing it are closed automatically by the widget restorer, so no migration is needed. - Request a section through notifications-ui-slice rather than widget config: an already-open widget keeps its original config. The user menu row opens the inbox and the popup's "View" does too, so a notification can no longer expand on a hidden section. - Remove the user-menu gear; settings are reached from the widget's own Settings tab. - Align the channel warning icon with its label and colour it with the colorWarning token via Icon's colorToken prop. - Header and spacing corrections: group the title and Send button in a Flex so Send sits beside the title (matching Bookmark Lists, Collections, Document Types), drop the header's bottom padding, and give the list 12px top padding via the marginSM/paddingSM scale. - Use the marginSM token for the profile avatar's top margin. Adds notifications.tab.inbox / notifications.tab.settings in all 7 locales and four reducer tests for the new section request. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
The public settings export needs a compatibility plan, and the new tab-request consumer lacks component-level coverage.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Moves notification settings into a tabbed Notifications widget alongside the inbox.
Changes:
- Adds Inbox/Settings tabs and section-request state.
- Consolidates widget registration, header, and send action.
- Updates translations, styling, popup navigation, and tests.
File summaries
| File | Description |
|---|---|
translations/studio.sv.yaml |
Adds Swedish tab labels. |
translations/studio.no.yaml |
Adds Norwegian tab labels. |
translations/studio.it.yaml |
Adds Italian tab labels. |
translations/studio.fr.yaml |
Adds French tab labels. |
translations/studio.es.yaml |
Adds Spanish tab labels. |
translations/studio.en.yaml |
Adds English tab labels. |
translations/studio.de.yaml |
Adds German tab labels. |
assets/js/src/core/modules/notifications/widget-configs.ts |
Removes the standalone settings widget config. |
assets/js/src/core/modules/notifications/user-menu-item.tsx |
Removes the settings gear and opens Inbox. |
assets/js/src/core/modules/notifications/settings/notification-settings.styles.ts |
Aligns warning icons. |
assets/js/src/core/modules/notifications/settings/notification-settings-view.tsx |
Applies warning color and alignment. |
assets/js/src/core/modules/notifications/settings/notification-settings-container.tsx |
Removes the redundant settings heading. |
assets/js/src/core/modules/notifications/notifications-widget.tsx |
Adds the consolidated tabbed widget. |
assets/js/src/core/modules/notifications/notifications-view.tsx |
Moves shared header and modal outward. |
assets/js/src/core/modules/notifications/notifications-ui-slice.ts |
Adds section navigation requests. |
assets/js/src/core/modules/notifications/notifications-ui-slice.test.ts |
Tests section-request reducer behavior. |
assets/js/src/core/modules/notifications/notification-popup/notification-popup-content.tsx |
Selects Inbox before expanding notifications. |
assets/js/src/core/modules/notifications/index.tsx |
Registers only the consolidated widget. |
assets/js/src/core/modules/app/base-layout/left-sidebar-view.styles.ts |
Uses tokenized avatar spacing. |
Review details
- Files reviewed: 19/19 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| import { NOTIFICATIONS } from './widget-configs' | ||
|
|
||
| export { NOTIFICATIONS, NOTIFICATION_SETTINGS } from './widget-configs' | ||
| export { NOTIFICATIONS } from './widget-configs' |
There was a problem hiding this comment.
Checked this and it isn't source-breaking — NOTIFICATION_SETTINGS was never reachable by third-party bundles, so a compatibility shim would be dead code.
The public federated entry point is ./modules/notifications → js/src/sdk/modules/notifications/index.ts (rsbuild.sdk.config.ts:144). That SDK barrel exports the notification list, the API slice, the dynamic-type definitions/registries and the payload utils — it has never re-exported widget-configs or this core index.tsx. NOTIFICATION_SETTINGS lived only in the internal core barrel behind the @Pimcore/* alias, which is resolved inside studio-ui's own build and is not available to bundles (collab-bundle, for example, has no @Pimcore alias in its rsbuild/tsconfig and imports exclusively from @pimcore/studio-ui-bundle/*).
So the removal is internal-only. Keeping the config as a deprecated alias would actually be worse than removing it: it is a WidgetManagerTabConfig pointing at component notification-settings, which is no longer registered, so openMainWidget(NOTIFICATION_SETTINGS) would hit the "Widget not found" path in widget-manager-factory.tsx rather than degrade gracefully.
Happy to reconsider if the SDK barrel is meant to re-export it — that would make it public and this a genuine break.
| useEffect(() => { | ||
| if (sectionRequest !== null) { | ||
| setActiveSection(sectionRequest.section) | ||
| dispatch(clearNotificationsSectionRequest()) | ||
| } |
| // Deep link from elsewhere in the app (the user menu's gear). An already-open widget keeps its | ||
| // original config, so a section cannot be requested through it — this signal is read live. |
…mment Addresses the Copilot review: - Add a NotificationsWidget component test. The reducer tests covered the slice but nothing exercised the consumer, so selecting the requested section and acknowledging the request could regress silently. The Tabs stub honours destroyInactiveTabPane, so the round-trip test fails if that prop is ever added here — verified by injecting it. - The section-request comment described the user menu's gear, which this branch removes; describe section requests generically instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
What
Notification settings were a second top-level widget, reachable only from a gear icon in the user menu. They are now a section of the Notifications widget, next to the inbox.
Why this shape
Studio already does section tabs inside a widget —
user-detail-tab.tsx(settings / workspaces / key-bindings / references),robots-txt,perspective-editor,classification-store-config. The alternatives were worse fits for this content:SIDEBAR_CONTENT_WIDTHS.largeof 432px. It would have needed a redesign into a narrow vertical form, not a move.The deciding constraint was toolbars: the inbox pages and clears, settings saves and discards. So each section renders its own
ContentLayout, and only the title and Send button are shared. The filter sidebar stays scoped to the inbox.Notable details
destroyInactiveTabPane(the opposite of theuser-detail-tabprecedent), because destroying the settings pane on a tab switch silently discards an unsaved draft.widget-manager-persistence.ts,widget-restorer.tsx), so users with the old Notification Settings tab open have it closed for them.notifications-ui-slice, not widget config —openMainWidgetonly selects an existing node and won't push new attributes into an open widget. This mirrors the existingrequestExpandNotificationpattern and its comment. The popup's "View" now also requests the inbox, so a notification can't expand on a hidden section.NOTIFICATION_SETTINGSis removed from the module barrel. Nothing in studio-ui, collab-bundle or copilot-bundle referenced it, but it was a public export — flagging in case a third-party bundle consumes it.Design-system alignment
Flexso Send sits beside the title, matching Bookmark Lists, Collections, Predefined Properties and Document Types.Headerdefaultsjustifytospace-between, which is why two direct children pushed apart.Icon'scolorTokenprop withcolorWarning— the token already used for warning icons inmodal.styles.tsx. Its wrapper span neededinline-flexbecause the tooltip's ref-holding span isn't a flex container, so the 16px icon div fell to the text baseline.marginSMfor the avatar's top margin,paddingSM(12px) for the list's top padding.Testing