Let bundles contribute entries to the profile menu - #4018
Merged
Conversation
The profile dropdown had no extension point, so a bundle wanting a personal, user-scoped entry could only fall back to the left-sidebar component slot. Adds a `user-menu` context-menu slot: providers render between "My profile" and "Log out", and their icons get the same wrapper as the built-in entries so third- party rows line up. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Notifications, My profile and Log out become providers like any third-party entry, so a bundle can position itself against them, reorder them, or hide one via overrideSlotProvider — an SSO-only or kiosk deployment has a reason to drop Log out. Logout sits at 9999 rather than a merely large number: the registry defaults a missing priority to 999, so anything lower loses its last place to a provider that omits one. The send-notification modal moves to the modal holder, which is what lets the notifications row leave UserMenu at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Verdict: Needs changes
Adds an extensible user-menu context-menu slot and migrates built-in entries to module-owned providers.
Changes:
- Registers notifications, profile, and logout providers.
- Uses the registry to compose the user menu.
- Moves notification widget configurations to avoid circular imports.
Assessment:
- Fixes the root cause at the correct boundary (
user-menu.tsx:51-71). - Existing exports and call sites remain covered; no breaking API change found.
- Provider priorities duplicate the exported configuration.
- Public slot documentation is not updated.
- No focused tests were added; comparable menu-provider seams currently lack tests.
- Runtime registration remains risky because provider changes can alter hook order.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
notifications/widget-configs.ts |
Extracts notification widget definitions. |
notifications/user-menu-item.tsx |
Adds the notifications menu provider. |
notifications/index.tsx |
Registers and re-exports notification functionality. |
auth/profile/user-menu-item.tsx |
Adds the profile provider. |
auth/logout-user-menu-item.tsx |
Adds the logout provider. |
auth/index.ts |
Registers authentication-owned providers. |
context-menu-config.ts |
Defines the public user-menu slot. |
user-menu.tsx |
Renders registry-provided entries. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The three providers repeated the numbers the config already declares, so the exported positioning contract could drift from the real order. Matches how the tree and grid providers read theirs. Documents the slot in the Context Menu Registry guide: no context object, the built-in provider names and priorities, the module-init constraint, and how to hide a built-in entry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Adds a
user-menucontext-menu slot so bundles can contribute an entry to it, and moves the built-in rows onto the same slot.The user menu hardcoded its four items and had no extension point.
ContextMenuRegistryalready models this — providers, priority,nullto hide — and both surfaces are antdDropdowns fedItemType[], so no new machinery is needed. First use of that registry on app chrome rather than on an element.contextMenuConfig.userMenu, slot nameuser-menuoverrideSlotProvider.user-menu__item-iconwrapperTwo things worth knowing:
getSlotProvidersdefaults a missing priority to 999 — anything lower loses last place to a provider that omits one.useModalHolder, which is what lets the notifications row leaveUserMenu.Providers must register at module init:
useContextMenuSlotcalls eachuseMenuItem()in amap, so a late registration shifts hook order.No test —
base-layouthas no component-test setup. Happy to add the harness if wanted.Consumer PR: pimcore/collab-bundle#89