[LWD] feat(contacts): render desktop contact edit and delete actions - #20281
Open
claudiiafg wants to merge 1 commit into
Open
[LWD] feat(contacts): render desktop contact edit and delete actions#20281claudiiafg wants to merge 1 commit into
claudiiafg wants to merge 1 commit into
Conversation
live-github-bot
Bot
added
desktop
live-github-bot
Bot
changed the title
Contributor
Web Tools Build Status
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds Desktop (LWD) contact-detail edit/delete UI by reusing the shared @features/flow-contacts MVVM logic and dialogs, while also improving “Me” contact detail rendering (default selection, display-name formatting, and “Add external address” CTA).
Changes:
- Shared contacts flow: introduce rename dialog/drawer + edit/delete/signer dialogs and an orchestration ViewModel for edit/delete flows.
- Desktop MVVM: wire shared edit/delete flows into the Contacts detail pane, mount dialogs, and add required i18n strings.
- Test/mocks updates: expand populated contacts fixtures and adjust list/detail/integration expectations accordingly.
Reviewed changes
Copilot reviewed 33 out of 33 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| features/flow/contacts/src/utils/resolveMeContactDisplayName.web.test.ts | Adds unit coverage for Me display-name formatting helper. |
| features/flow/contacts/src/utils/resolveMeContactDisplayName.ts | Implements Me vs saved-contact display-name resolution. |
| features/flow/contacts/src/utils/index.ts | Re-exports new utils/constants. |
| features/flow/contacts/src/utils/constants.ts | Introduces default “Me” contact name constant. |
| features/flow/contacts/src/steps/List/ContactsListView.web.test.tsx | Updates list test expectations for expanded mock contacts. |
| features/flow/contacts/src/steps/EditContact/web.ts | Exposes the web rename dialog entrypoint. |
| features/flow/contacts/src/steps/EditContact/useRenameContactViewModel.ts | Hook wrapping rename controller/view-model + save action. |
| features/flow/contacts/src/steps/EditContact/useRenameContactDialogViewModel.ts | Dialog-level VM: open/close, draft name, saving state, confirm flow. |
| features/flow/contacts/src/steps/EditContact/types.ts | Types for rename VM, dialog/drawer props, and labels. |
| features/flow/contacts/src/steps/EditContact/native.ts | Exposes the native rename drawer entrypoint. |
| features/flow/contacts/src/steps/EditContact/model/viewModel.web.test.ts | Unit tests for rename VM validation/enablement logic. |
| features/flow/contacts/src/steps/EditContact/model/viewModel.ts | Pure rename VM computation (validation + confirm enablement). |
| features/flow/contacts/src/steps/EditContact/model/controller.ts | Controller bridging VM + domain validation + edit port. |
| features/flow/contacts/src/steps/EditContact/index.ts | Barrel exports for rename step/view-model/controller/types. |
| features/flow/contacts/src/steps/EditContact/ContactsRenameContactDrawer.native.tsx | Native bottom-sheet UI for rename contact. |
| features/flow/contacts/src/steps/EditContact/ContactsRenameContactDialog.web.tsx | Web dialog UI for rename contact. |
| features/flow/contacts/src/steps/Detail/web.ts | Exports new detail actions + delete/signer dialogs for web. |
| features/flow/contacts/src/steps/Detail/useContactDetailEditDeleteFlowViewModel.ts | Orchestrates edit/delete flow state (actions menu, signer gate, delete lifecycle). |
| features/flow/contacts/src/steps/Detail/types.ts | Adds types for detail actions labels + optional detail actions prop. |
| features/flow/contacts/src/steps/Detail/native.ts | Exports native detail menu + delete/signer dialogs. |
| features/flow/contacts/src/steps/Detail/index.ts | Re-exports new flow VM + updated type exports. |
| features/flow/contacts/src/steps/Detail/ContactDetailView.web.tsx | Renders new detail actions UI when provided. |
| features/flow/contacts/src/steps/Detail/ContactDetailView.web.test.tsx | Updates tests for Me name formatting and external address CTA. |
| features/flow/contacts/src/steps/Detail/components/ContactsEditSignerDialog/types.ts | Types for signer confirmation dialog/drawer. |
| features/flow/contacts/src/steps/Detail/components/ContactsEditSignerDialog/ContactsEditSignerDialog.web.tsx | Web signer-confirmation dialog UI. |
| features/flow/contacts/src/steps/Detail/components/ContactsEditSignerDialog/ContactsEditSignerDialog.native.tsx | Native signer-confirmation drawer UI. |
| features/flow/contacts/src/steps/Detail/components/ContactsDeleteContactDialog/types.ts | Types for delete confirmation dialog/drawer. |
| features/flow/contacts/src/steps/Detail/components/ContactsDeleteContactDialog/ContactsDeleteContactDialog.web.tsx | Web delete confirmation dialog UI. |
| features/flow/contacts/src/steps/Detail/components/ContactsDeleteContactDialog/ContactsDeleteContactDialog.native.tsx | Native delete confirmation drawer UI. |
| features/flow/contacts/src/steps/Detail/components/ContactDetailHeader.web.tsx | Uses Me display-name resolver and adjusts CTA label for Me. |
| features/flow/contacts/src/steps/Detail/components/ContactDetailActionsMenu/ContactDetailActionsMenu.native.tsx | Native actions menu (edit/delete). |
| features/flow/contacts/src/steps/Detail/components/ContactDetailActions/ContactDetailActions.web.tsx | Web actions buttons (edit/delete icons). |
| features/flow/contacts/src/index.ts | Exports new EditContact step for web consumers. |
| features/flow/contacts/src/index.native.ts | Exports new EditContact step for native consumers. |
| domain/entity/contact/src/schema.test.ts | Updates mock-contacts expectations after fixture expansion. |
| domain/entity/contact/src/schema.mock.ts | Expands populated contacts and adds Me-with-addresses mock. |
| apps/ledger-live-desktop/static/i18n/en/app.json | Adds English strings for new contact detail actions + dialogs. |
| apps/ledger-live-desktop/src/renderer/screens/settings/sections/Developer/ContactsDevTool/hooks/tests/useContactsDevToolViewModel.test.ts | Updates devtool test expectations for expanded fixtures. |
| apps/ledger-live-desktop/src/mvvm/features/Contacts/screens/Contacts/useContactsViewModel.ts | Plumbs edit/delete dialogs data into page VM. |
| apps/ledger-live-desktop/src/mvvm/features/Contacts/screens/Contacts/useContactDetailPaneAdapter.ts | Defaults detail pane to Me; wires detail actions; adds Me display-name/CTA labels. |
| apps/ledger-live-desktop/src/mvvm/features/Contacts/screens/Contacts/useContactDetailEditDeleteAdapter.ts | Desktop adapter mapping shared flow VMs to concrete dialog props + i18n labels. |
| apps/ledger-live-desktop/src/mvvm/features/Contacts/screens/Contacts/ContactsView.tsx | Mounts rename/delete/signer dialogs on the screen. |
| apps/ledger-live-desktop/src/mvvm/features/Contacts/hooks/useContactsEditDeletePorts.ts | Provides redux-backed ports for rename/delete operations. |
| apps/ledger-live-desktop/src/mvvm/features/Contacts/integrations/Contacts.integration.test.tsx | Updates integration coverage for default Me detail rendering and expanded fixtures. |
| .changeset/lwd-me-contact-detail.md | Declares minor bumps for affected packages and describes the Me detail default/rendering update. |
claudiiafg
force-pushed
the
feat/LIVE-33943_web-edit
branch
from
July 31, 2026 11:10
6d89c5f to
1cfcfb4
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 26 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
features/flow/contacts/src/steps/Detail/types.ts:61
ContactDetailLabelsno longer includesaddExternalAddressandformatMeDisplayName, but they are still consumed (e.g.ContactDetailHeader.web.tsxuses both and the Desktop adapter still provides them). This makes the type definition inconsistent with current consumers and will break TypeScript builds.
export type ContactDetailLabels = Readonly<{
addAddress: string;
addYourAddress?: string;
emptyMeTitle: string;
emptyContactTitle: (name: string) => string;
features/flow/contacts/src/steps/EditContact/useRenameContactViewModel.ts:17
setDraftNameis declared but never used. With typical lint settings (no-unused-vars), this will fail CI for an unused parameter. Either remove it (and update callers) or prefix it with_to mark it intentionally unused.
contactId: ContactId,
currentName: string,
draftName: string,
setDraftName: (draftName: string) => void,
editPort: ContactEditPort,
Contributor
Rsdoctor Bundle Diff AnalysisFound 7 projects in monorepo, 7 projects with changes. 📊 Quick Summary
📋 Detailed Reports (Click to expand)📁 desktop-mainPath:
📁 desktop-rendererPath:
📁 desktop-preloaderPath:
📁 desktop-webviewDappPreloaderPath:
📁 desktop-webviewPreloaderPath:
📁 desktop-workersPath:
📁 mobilePath:
Generated by Rsdoctor GitHub Action |
claudiiafg
force-pushed
the
feat/LIVE-33943_web-edit
branch
from
July 31, 2026 15:08
1cfcfb4 to
e4ad405
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 27 changed files in this pull request and generated no new comments.
Suppressed comments (4)
features/flow/contacts/src/steps/Detail/types.ts:61
ContactDetailLabelsno longer includesaddExternalAddress/formatMeDisplayName, but those fields are still used byContactDetailHeader.web.tsxand theContactDetailViewweb tests. As-is, this type change breaks consumers; either keep these optional label fields or update all call sites accordingly.
export type ContactDetailLabels = Readonly<{
addAddress: string;
addYourAddress?: string;
emptyMeTitle: string;
emptyContactTitle: (name: string) => string;
features/flow/contacts/src/steps/EditContact/useRenameContactViewModel.ts:16
setDraftNameis accepted as a parameter but never used. With typicalnoUnusedParameterssettings this will fail lint/typecheck; consider removing it from the hook signature (and updating call sites) or prefixing with_if intentionally unused.
export function useRenameContactViewModel(
contactId: ContactId,
currentName: string,
draftName: string,
setDraftName: (draftName: string) => void,
apps/ledger-live-desktop/src/mvvm/features/Contacts/screens/Contacts/useContactDetailEditDeleteAdapter.ts:59
confirmNameis included in the rename labels here, butContactsRenameContactDialogdoesn’t read/use it (it rendersapplyChangesinstead). Consider removingconfirmNamefrom the rename label type and i18n wiring, or use it in the UI to avoid carrying unused translation keys/props.
title: t("contacts.editContact.title"),
namePlaceholder: t("contacts.editContact.namePlaceholder"),
namingDisclaimer: t("contacts.editContact.namingDisclaimer"),
confirmName: t("contacts.editContact.confirmName"),
applyChanges: t("contacts.editContact.applyChanges"),
nameValidationErrors: {
apps/ledger-live-desktop/src/mvvm/features/Contacts/screens/Contacts/useContactDetailEditDeleteAdapter.ts:36
- The new edit/delete adapter wiring introduces user-facing behavior (rendering detail actions, opening rename/delete/signer dialogs, and delete success navigation). There’s already a Contacts integration test suite for Desktop; please add/extend tests to cover at least: opening the actions, launching rename/delete dialogs, and the delete-success reset to the Me contact.
This issue also appears on line 54 of the same file.
export function useContactDetailEditDeleteAdapter(
contactId: ContactId | undefined,
onDeleteSuccess: () => void,
): ContactDetailEditDeleteDialogProps {
const { t } = useTranslation();
const ports = useContactsEditDeletePorts();
const flow = useContactDetailEditDeleteFlowViewModel({
contactId: contactId ?? "contact-me",
ports,
onDeleteSuccess,
});
claudiiafg
force-pushed
the
feat/LIVE-33943_web-edit
branch
from
July 31, 2026 15:23
e4ad405 to
cab5ec1
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 31 out of 31 changed files in this pull request and generated no new comments.
Suppressed comments (1)
features/flow/contacts/src/steps/EditContact/types.ts:33
ContactsRenameContactLabelsincludesconfirmName, but the rename dialog implementation (ContactsRenameContactDialog.web.tsx) never reads it (it useslabels.applyChangesfor the CTA). This forces consumers (e.g. desktop adapter + i18n) to provide an extra label that is effectively dead and can easily drift/out-of-sync.
Consider removing confirmName from ContactsRenameContactLabels (and corresponding i18n + adapter wiring), or switching the dialog CTA to use confirmName and drop applyChanges to keep a single source of truth for the button label.
export type ContactsRenameContactLabels = Readonly<{
title: string;
namePlaceholder: string;
namingDisclaimer: string;
confirmName: string;
applyChanges: string;
nameValidationErrors: Readonly<Record<ContactNameValidationErrorName, string>>;
claudiiafg
force-pushed
the
feat/LIVE-33943_web-edit
branch
from
July 31, 2026 15:44
cab5ec1 to
8940392
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 31 out of 31 changed files in this pull request and generated no new comments.
Suppressed comments (1)
features/flow/contacts/src/steps/EditContact/types.ts:33
ContactsRenameContactLabelsincludesconfirmName, but the rename dialog UI useslabels.applyChangesfor the primary button and there are no other usages ofconfirmNamein this step. Keeping an unused label increases API surface and i18n burden; consider removingconfirmNamefrom the labels type and the corresponding i18n wiring, or switch the UI to use it instead ofapplyChanges.
export type ContactsRenameContactLabels = Readonly<{
title: string;
namePlaceholder: string;
namingDisclaimer: string;
confirmName: string;
applyChanges: string;
nameValidationErrors: Readonly<Record<ContactNameValidationErrorName, string>>;
claudiiafg
force-pushed
the
feat/LIVE-33943_web-edit
branch
from
August 3, 2026 08:18
8940392 to
3ddd952
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 31 out of 31 changed files in this pull request and generated no new comments.
Suppressed comments (2)
features/flow/contacts/src/steps/Detail/components/ContactsDeleteContactDialog/ContactsDeleteContactDialog.web.tsx:58
- The delete confirm button stays clickable while
isDeletingis true (only shows a loading spinner). This can allow multiple rapid clicks and triggeronConfirm()multiple times; please disable the confirm button while deleting, consistent with the cancel button and other dialogs.
<Button
appearance="red"
size="lg"
isFull
loading={isDeleting}
onClick={() => void onConfirm()}
data-testid="contacts-delete-contact-confirm"
features/flow/contacts/src/steps/EditContact/types.ts:33
ContactsRenameContactLabelsrequires aconfirmNamelabel, but the current rename dialog usesapplyChangesand there is no drawer/component consumingconfirmName. This adds unused translation keys and plumbing (e.g.contacts.editContact.confirmName). Consider removingconfirmNamefrom the labels type + i18n, or switching the UI to use it consistently.
export type ContactsRenameContactLabels = Readonly<{
title: string;
namePlaceholder: string;
namingDisclaimer: string;
confirmName: string;
applyChanges: string;
nameValidationErrors: Readonly<Record<ContactNameValidationErrorName, string>>;
Contributor
|
| import type { ContactDetailActionsPorts } from "@features/flow-contacts"; | ||
| import { useDispatch, useStore } from "LLD/hooks/redux"; | ||
|
|
||
| export function useContactsEditDeletePorts(): ContactDetailActionsPorts { |
Contributor
There was a problem hiding this comment.
[ASK] "Ports" is for what? :)
Contributor
Author
There was a problem hiding this comment.
Ports are dependency-injection interfaces that keep @features/flow-contacts platform-agnostic.
The shared flow defines what it needs (ContactEditPort.renameContact, ContactDeletionPort.deleteContact) without knowing how it’s persisted.
useContactsEditDeletePorts is the desktop wiring layer.
Contributor
There was a problem hiding this comment.
You duplicate all 3 dialogs, you can have a main shell for the dialog and then inject the content with components ?
Contributor
Author
There was a problem hiding this comment.
what do you think of doing this a follow up?
I don't want to make this PR any bigger 😅
Contributor
There was a problem hiding this comment.
[ASK] Do we need both controller and viewModel? they have the same purpose usually like (MVC or MVVM)
Contributor
Author
There was a problem hiding this comment.
I believe yes, the controller.ts is a bridge exposes the pure VM via getViewModel() and handles save() by calling the injected editPort.
claudiiafg
force-pushed
the
feat/LIVE-33943_web-edit
branch
from
August 3, 2026 09:09
3ddd952 to
1bb5f12
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 31 out of 31 changed files in this pull request and generated no new comments.
Suppressed comments (2)
apps/ledger-live-desktop/src/mvvm/features/Contacts/screens/Contacts/useContactsViewModel.ts:28
useContactDetailEditDeleteAdapteris imported as a runtime value but only used for aReturnTypeinContactsPageViewModel. This will emit an unnecessary JS import (and can introduce unwanted runtime coupling). Prefer importing the exported props type instead and using a type-only import.
import { useContactDetailEditDeleteAdapter } from "./useContactDetailEditDeleteAdapter";
features/flow/contacts/src/steps/EditContact/useRenameContactViewModel.ts:10
- The
savereturn type uses an inlineimport("@domain/entity-contact").Contacteven though this file already imports from@domain/entity-contact. For consistency with other view models in this package (e.g.useAddContactViewModel), importContactas a type and reference it directly.
export type UseRenameContactViewModelResult = RenameContactViewModel &
Readonly<{
save: () => Promise<import("@domain/entity-contact").Contact>;
}>;
claudiiafg
force-pushed
the
feat/LIVE-33943_web-edit
branch
from
August 3, 2026 09:27
1bb5f12 to
ddfa3a2
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 31 out of 31 changed files in this pull request and generated no new comments.
Suppressed comments (3)
apps/ledger-live-desktop/src/mvvm/features/Contacts/hooks/useContactsEditDeletePorts.ts:10
- Import order is inconsistent with the other MVVM hooks in this feature (they import React first, then domain/app modules). Keeping React imports first improves consistency and reduces merge churn.
import {
deleteContact as deleteContactAction,
parseContactName,
renameContact as renameContactAction,
selectContactById,
} from "@domain/entity-contact";
import { useMemo } from "react";
import type { ContactDetailActionsPorts } from "@features/flow-contacts";
import { useDispatch, useStore } from "LLD/hooks/redux";
features/flow/contacts/src/steps/Detail/web.ts:7
- PR description mentions "ContactsDeleteContactDialog / ContactsEditSignerDialog (web + native exports)", but the added components are web-only and
steps/Detail/native.tshas no matching exports. If native support is intended, add native implementations + exports; otherwise please update the PR description to avoid confusion for reviewers/consumers.
export { ContactDetailActions } from "./components/ContactDetailActions/ContactDetailActions.web";
export type { ContactDetailActionsProps } from "./components/ContactDetailActions/ContactDetailActions.web";
export { ContactsDeleteContactDialog } from "./components/ContactsDeleteContactDialog/ContactsDeleteContactDialog.web";
export { ContactsEditSignerDialog } from "./components/ContactsEditSignerDialog/ContactsEditSignerDialog.web";
export type { ContactsDeleteContactDialogProps } from "./components/ContactsDeleteContactDialog/types";
features/flow/contacts/src/steps/EditContact/useRenameContactViewModel.ts:10
- The
savereturn type uses an inlineimport("@domain/entity-contact")type. This is the only occurrence in the contacts flow and makes the public API harder to read; prefer importingContactas a type like the existing add-contact view model does.
export type UseRenameContactViewModelResult = RenameContactViewModel &
Readonly<{
save: () => Promise<import("@domain/entity-contact").Contact>;
}>;
|
mcayuelas-ledger
approved these changes
Aug 3, 2026
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
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.
📝 Description
Renders edit and delete contact actions on Desktop contact detail. Adds the shared
@features/flow-contactsedit/delete UI and wires it in the Desktop MVVM layer. Consumes existing shared state (useContactDetailActionsViewModel, delete lifecycle, signer-required flag) without reimplementing Contacts rules.Changes
Shared (
@features/flow-contacts)EditContactstep — rename dialog, validation, apply-changes flowuseContactDetailEditDeleteFlowViewModel— orchestrates edit, delete, and signer statesContactsDeleteContactDialog/ContactsEditSignerDialog(web + native exports)ContactDetailActions— pencil + trash icon buttons on detail pane (delete hidden for Me)Desktop (
ledger-live-desktop)renameContact/deleteContactuseContactDetailEditDeleteAdapter+ pane adapter wiringContactsViewBehaviour
Screen.Recording.2026-07-31.at.16.06.20.mov
🔗 Context
LIVE-33943