Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Adds a “Remove member” action to the Note Settings team list, wiring UI → application/domain → repository API to remove a collaborator (with confirmation) and refresh the displayed settings.
Changes:
- Add a per-member “More actions” context menu with a “Remove” option and confirmation flow.
- Bubble a “team member removed” event up to NoteSettings to reload note settings/team.
- Add
removeMemberByUserIdthrough repository/service/composable layers and introduce new i18n strings.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/presentation/pages/NoteSettings.vue | Listens for member-removed event and reloads note settings. |
| src/presentation/components/team/Team.vue | Renders new per-member actions component and re-emits removal event upward. |
| src/presentation/components/team/MoreActions.vue | New UI component showing context menu + confirmation + remove call. |
| src/infrastructure/noteSettings.repository.ts | Adds DELETE call to remove a team member by userId. |
| src/domain/noteSettings.service.ts | Exposes removeMemberByUserId in domain service. |
| src/domain/noteSettings.repository.interface.ts | Extends repository interface with removeMemberByUserId. |
| src/application/services/useNoteSettings.ts | Adds composable method for removing a team member. |
| src/application/i18n/messages/en.json | Adds menu label and confirmation copy for removal. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <template> | ||
| <button | ||
| ref="triggerButton" | ||
| class="more-actions-button" |
There was a problem hiding this comment.
The icon-only needs an accessible name and should explicitly set type="button" to avoid default submit behavior when used inside a . Consider adding an aria-label/title (e.g., "More actions") and type="button" on this button element.
| class="more-actions-button" | |
| class="more-actions-button" | |
| type="button" | |
| :aria-label="t('noteSettings.team.moreActions')" | |
| :title="t('noteSettings.team.moreActions')" |
| onActivate: () => { | ||
| handleRemove(props.teamMember); | ||
| hide(); |
There was a problem hiding this comment.
This callback triggers the async removal flow but does not await or handle errors from handleRemove(). If removeMemberByUserId throws (network/403/etc.), this can become an unhandled promise rejection and the user gets no feedback. Please make the activation path handle the promise (e.g., mark onActivate async and await, or attach a .catch that shows an error).
| onActivate: () => { | |
| handleRemove(props.teamMember); | |
| hide(); | |
| onActivate: async () => { | |
| hide(); | |
| try { | |
| await handleRemove(props.teamMember); | |
| } catch (error) { | |
| console.error('Failed to remove team member', error); | |
| } |
| <MoreActions | ||
| :note-id="noteId" | ||
| :team-member="member" | ||
| @team-member-removed="handleMemberRemoved" | ||
| /> |
There was a problem hiding this comment.
MoreActions is rendered for every team member without any permission/self/creator checks. Per the PR description, users should only be able to remove other members and only when they have "write" access; additionally, creator/self removal typically should be blocked in UI. Consider conditionally rendering/disabling this component based on current user role/ID and note creator ID (similar to RoleSelect).
| }, | ||
| "removeMemberConfirmationTitle": "Remove member", | ||
| "removeMemberConfirmationBody": "Are you sure you want to remove user from the team?", | ||
| "ContextMenu": { |
There was a problem hiding this comment.
The i18n key "ContextMenu" uses PascalCase, which is inconsistent with the surrounding noteSettings.team keys (mostly lowerCamelCase). Consider renaming it to "contextMenu" (or similar) and updating usages (e.g., t('noteSettings.team.contextMenu.remove')) to keep the messages structure consistent.
| "ContextMenu": { | |
| "contextMenu": { |
| "Write": "Writer" | ||
| }, | ||
| "removeMemberConfirmationTitle": "Remove member", | ||
| "removeMemberConfirmationBody": "Are you sure you want to remove user from the team?", |
There was a problem hiding this comment.
Grammar: the confirmation text is missing an article and reads a bit awkwardly.
| "removeMemberConfirmationBody": "Are you sure you want to remove user from the team?", | |
| "removeMemberConfirmationBody": "Are you sure you want to remove the user from the team?", |
| import useNoteSettings from '@/application/services/useNoteSettings'; | ||
|
|
||
| const { removeMemberByUserId } = useNoteSettings(); |
There was a problem hiding this comment.
MoreActions is instantiated per team member row, but it calls useNoteSettings(), which creates its own composable state (noteSettings refs, parentNote refs, router, etc.) per instance. This is unnecessary overhead and can lead to duplicated state; consider passing removeMemberByUserId down from the parent, or extracting a lighter-weight service call that doesn't allocate the full composable state for each row.
| /** | ||
| * Handle team member removal by refreshing the note settings | ||
| */ | ||
| async function handleTeamMemberRemoved() { |
There was a problem hiding this comment.
i think that instead of reloading the page completely we could just update noteSettings composable
then we would need to handle boolean that is returned by the api endpoint
2 participants
Added the ability to remove a team member from a note.
Users with 'write' access to a note can now remove other members from its team.
In the note's settings menu, a new "Remove" option has been added to the context menu (located right after role selector). This action requires confirmation before the member is actually removed.