feat(database): match desktop's relation creation dialog - #482
Merged
Conversation
Ports `relation_creation_dialog.dart` so adding a Relation property behaves and reads the same on both clients. Behaviour: - Picking a target database now names the property after it and the reciprocal after the source, mirroring `_RelatedToSection.onSelect`. - Self-relations are first class: the trigger reads "This database" with its icon suppressed, the name fields use the "Related X" / "Related back to X" copy, and the reciprocal label becomes "Inverse related property name". - The target list moved behind a dropdown trigger, matching `AFDropDownMenu` in its non-input mode (which has no search — `updateFilteredItems` short-circuits to the full list when `isInput` is false). - Added the property icon picker desktop has, wired to the existing `useUpdatePropertyIconDispatch`. - Dropped the summary panel; desktop's dialog has no such section. Styling follows the appflowy_ui tokens, which map 1:1 onto the web scale (spacing 4/6/8/12/16/20, radius m = 8, body 14/400, heading4 16/700). Two icons with no web equivalent were ported and converted to currentColor. The dialog was previously English-only: every string came from an inline `defaultValue`. Added the 14 `grid.relation.*` keys that already ship translated on desktop and removed the literals. This changes visible copy where web had invented its own — notably "One page" becomes "1 page only". Both Playwright helpers clicked candidates directly, so they now open the dropdown first. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
NormalModal still used MUI buttons while the rest of the app moved to the token-based `Button`, whose `default`/`outline` variants are already exact ports of `AFFilledTextButton.primary` / `AFOutlinedTextButton.normal`. Only the metrics were off, so the footer now also applies desktop's `AFButtonSize.m` pairing (px-4, font-medium) instead of the tighter default. This touches every modal in the app, so the swap was checked first: no caller passes MUI-specific props to `okButtonProps`/`cancelButtonProps` — only `disabled`, `className` and `data-testid`. Caller classNames are merged rather than overwritten so `HIDDEN_BUTTON_PROPS` still hides footers, and `type="button"` is set explicitly because MUI defaulted to it and the design-system button does not (a bare button inside a form would submit). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reviewer's GuideAligns the web Relation Creation dialog and shared modal footer with the Flutter/desktop implementation: introduces a new AF-style dropdown UI for database/limit selection, updates naming and self-relation behavior, wires in icon selection and full i18n keys, and replaces MUI buttons in NormalModal with design-system Button components, along with updated tests and Playwright helpers. Sequence diagram for the updated relation creation flowsequenceDiagram
actor User
participant RelationCreationDialog
participant AFDropdown as AFDropdown
participant NormalModal as NormalModal
participant IconDispatch as useUpdatePropertyIconDispatch
participant OnCreate as onCreate
User->>RelationCreationDialog: open (open=true, fieldId, initialFieldName)
RelationCreationDialog->>IconDispatch: useUpdatePropertyIconDispatch(fieldId)
User->>RelationCreationDialog: click relation-field-icon-trigger
RelationCreationDialog->>IconDispatch: updateIcon(iconValue)
User->>AFDropdown: click relation-database-trigger
AFDropdown-->>RelationCreationDialog: show candidates
User->>AFDropdown: click relation-candidate-databaseId
AFDropdown-->>RelationCreationDialog: handleSelectCandidate(candidate)
RelationCreationDialog->>RelationCreationDialog: setSelectedDatabaseId
RelationCreationDialog->>RelationCreationDialog: setFieldName / setReciprocalFieldName
AFDropdown-->>User: close dropdown
User->>RelationCreationDialog: toggle relation-two-way-switch
RelationCreationDialog->>RelationCreationDialog: setIsTwoWay(true)
RelationCreationDialog->>RelationCreationDialog: show reciprocalFieldName input
User->>NormalModal: click modal-ok-button
NormalModal->>RelationCreationDialog: onOk()
RelationCreationDialog->>OnCreate: onCreate(RelationCreationResult)
RelationCreationDialog->>RelationCreationDialog: reset local state
RelationCreationDialog-->>User: dialog closed via onOpenChange(false)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
🥷 Ninja i18n – 🛎️ Translations need to be updatedProject
|
| lint rule | new reports | level | link |
|---|---|---|---|
| Missing translation | 504 | warning | contribute (via Fink 🐦) |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The new
grid.relation.selectADatabsei18n key appears to be misspelled; consider renaming it (and its usages) to something likegrid.relation.selectADatabasebefore this typo propagates across clients. - In
RelationCreationDialog.test.tsxthe mock forCustomIconPopovertypeschildrenasReactNodebut does not importReactNode; adding the import (or dropping the explicit type) would avoid type/check-time issues. - The custom
AFDropdownimplementation currently only closes on mouse clicks and lacks Escape/keyboard navigation handling; if you want to keep parity with other menus and improve accessibility, consider adding basic keyboard support.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `grid.relation.selectADatabse` i18n key appears to be misspelled; consider renaming it (and its usages) to something like `grid.relation.selectADatabase` before this typo propagates across clients.
- In `RelationCreationDialog.test.tsx` the mock for `CustomIconPopover` types `children` as `ReactNode` but does not import `ReactNode`; adding the import (or dropping the explicit type) would avoid type/check-time issues.
- The custom `AFDropdown` implementation currently only closes on mouse clicks and lacks Escape/keyboard navigation handling; if you want to keep parity with other menus and improve accessibility, consider adding basic keyboard support.
## Individual Comments
### Comment 1
<location path="src/components/database/components/property/relation/RelationCreationDialog.tsx" line_range="513" />
<code_context>
+ </span>
+ </>
+ ) : (
+ <span className='min-w-0 flex-1 truncate text-text-tertiary'>{t('grid.relation.selectADatabse')}</span>
+ )
+ }
</code_context>
<issue_to_address>
**issue (typo):** Fix the typo in the `selectADatabse` i18n key to avoid inconsistent naming
The `grid.relation.selectADatabse` key is misspelled, which makes it harder to discover and reuse (e.g., via search). Please rename it to something like `grid.relation.selectADatabase` and update the translation bundle accordingly.
Suggested implementation:
```typescript
<span className='min-w-0 flex-1 truncate text-text-tertiary'>{t('grid.relation.selectADatabase')}</span>
```
You will also need to update your i18n translation bundle(s), for example:
- In the relevant locale JSON/YAML (or TS) file(s), rename the key `grid.relation.selectADatabse` to `grid.relation.selectADatabase`, or add `grid.relation.selectADatabase` with the same value and remove the old one.
- If there are any other usages of `grid.relation.selectADatabse` elsewhere in the codebase, update them to `grid.relation.selectADatabase` for consistency.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| </span> | ||
| </> | ||
| ) : ( | ||
| <span className='min-w-0 flex-1 truncate text-text-tertiary'>{t('grid.relation.selectADatabse')}</span> |
There was a problem hiding this comment.
issue (typo): Fix the typo in the selectADatabse i18n key to avoid inconsistent naming
The grid.relation.selectADatabse key is misspelled, which makes it harder to discover and reuse (e.g., via search). Please rename it to something like grid.relation.selectADatabase and update the translation bundle accordingly.
Suggested implementation:
<span className='min-w-0 flex-1 truncate text-text-tertiary'>{t('grid.relation.selectADatabase')}</span>You will also need to update your i18n translation bundle(s), for example:
- In the relevant locale JSON/YAML (or TS) file(s), rename the key
grid.relation.selectADatabsetogrid.relation.selectADatabase, or addgrid.relation.selectADatabasewith the same value and remove the old one. - If there are any other usages of
grid.relation.selectADatabseelsewhere in the codebase, update them togrid.relation.selectADatabasefor consistency.
The creation dialog was ported but the property menu still diverged from `type_option_editor/relation.dart`. Rebuilt it row for row: - Every row now carries its leading icon in `iconColorScheme.primary`: `database_s` for the data source, `count_s` for the limit, and the two-way glyph for the reciprocal toggle. - The database row gained its "Data source" title; the database name moved into the trailing slot as `caption.standard` in `textColorScheme.secondary`, preceded by the target's icon. - Two-way relation is a submenu showing On/Off, not an inline switch. The submenu is desktop's `_TwoWayRelationPopoverContent`: an "Enable" toggle, a divider, then the reciprocal name field, disabled while two-way is off and debounced on write (empty input is ignored, as on desktop). - The reciprocal name now seeds from the relation field's own name rather than the source database name, matching `initState`/`onTwoWayToggle`. - Dropped the disable-two-way confirmation dialog; desktop writes `isTwoWay: false` directly, and the submenu is already a two-step interaction. - The section header is `caption.standard` (12px/400), not 12px/500. Also fixes the Relation field-type icon: `relation.svg` held desktop's *two-way* glyph (two crossing arrows), so every relation column header and the type row rendered the wrong symbol. Replaced with desktop's one-way `relation.svg` geometry — a single diagonal arrow. Verified in the browser against the running stack, not just in unit tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The data-source picker listed the *registered* database views, which are the inner views and are almost always named "Grid" — so nearly every entry read "Grid" instead of the database's name. `useRelationData` already resolved the container name, but only for the currently selected view, never for the list. Resolve candidates once from the folder outline the hook already loads: - `databaseCandidates` pairs each `(databaseId, viewId)` with the container that owns the user-facing name, and with the registered view's ancestors. - The picker renders desktop's `_DatabaseListItem`: icon, name, and the ancestor path as a two-line subtitle, elided to `first / ... / last` when deeper than two, with the tick on the selected row. - Added the search field from `_DatabaseList`, filtering on the database name, falling back to "No results". Resolution reuses the cached outline via `findAncestors`, so no extra fetches are needed — unlike the creation dialog, which has no outline to hand and batches its own view lookups. Verified against the running stack: the list now reads "employees / General / employees", "5000_employees / Big Database / 5000_employees", and search narrows it correctly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two issues from a pass with the Vercel React performance rules: `js-index-maps` — `findAncestors` restarts from the outline root on every call, so resolving N databases re-walked the whole tree N times. Collect the registered view ids into a Set up front and record ancestry during a single traversal, copying the path only for the handful of matching nodes. `rerender-derived-state-no-effect` — the reciprocal name was mirrored from the type option into state inside an effect, costing an extra commit per change and needing an `eslint-disable` that hid `fieldName` from the dependencies. Adjust during render instead: keep a nullable draft for the in-flight edit and fall back to the persisted value, so an external change (our own debounced write, or a remote edit) is picked up without an effect. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`AvatarFallback` derived its colour from `children` whenever that was a
string, and only fell back to the `name` prop otherwise. Several call sites
render just the initial, so the colour came from hashing one character:
"Nathan Foo" hashes to badge 7, "N" to badge 19, meaning the same person
appeared olive in a grid cell and a different colour in the person filter
menu. Call sites that passed `name` explicitly were silently overridden too —
MembersPanel supplied `name={m.name}` but rendered the initial as its child,
so the prop never took effect.
Make `name` authoritative, matching desktop's `AFAvatar`, where the colour
always comes from `colorHash ?? name` and the initial is only ever the
rendered glyph. Then pass the full name at the three sites that had none:
the person filter menu, the advanced filter panel, and the row-comment
member avatar.
The hash itself already matches desktop byte for byte, including the
workspace icon, which runs the same `randomColorFromDesignSystem` over the
workspace name and lands on the same badge slot.
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
What
Brings the Add relation property flow in line with the Flutter client
(
relation_creation_dialog.dart). Behaviour, layout, tokens and copy were alldiffed against desktop rather than approximated.
Behaviour changes
Related X/Related back to Xcopy, "Inverse related property name" labelOn the dropdown having no search:
AFDropDownMenu.updateFilteredItemsshort-circuits to the full list whenever
isInputis false, and this dialogconstructs it without that flag — so desktop genuinely has no search here.
Styling
The appflowy_ui tokens map 1:1 onto the web scale —
tailwind.config.cjsuses the same 100–600 ramp (4/6/8/12/16/20) as
AppFlowySpacing/AppFlowyBorderRadius, andtailwind/new-colors.cjsis generated from thesame token JSON as the Flutter theme. So
textColorScheme.primary→text-text-primary,fillColorScheme.contentHover→bg-fill-content-hover,shadow.small→shadow-popover, and so on.Ported: 420px width, left-aligned
heading4.prominenttitle, section labelsas
body.standardontext-primary(not muted captions),AFTextField(size: m)metrics, the_DropdownMenuButton7px/5px border inset, theAFPopoverlayer-01 + shadow panel, and hover-only item backgrounds —
DatabaseMetaItemand
LimitOptionItemsupply custom builders, so neither tints the selectedrow; the limit list marks selection with a tick instead.
Two icons had no web equivalent and were ported, converted to
currentColor:two_way_relation.svgandcount.svg.Localization
The dialog was effectively English-only — every string came from an inline
defaultValue, so no locale ever translated it. Added the 14grid.relation.*keys that already ship translated on desktop and droppedthe literals.
This changes visible copy where web had invented its own wording:
limitOnePageShared modal footer
Second commit swaps
NormalModal's MUI buttons for the token-basedButton,whose variants are already exact ports of the AF button specs; only padding
and weight were off. This touches all 49 modal call sites, so it was checked
first — none pass MUI-specific props, caller
classNames are merged ratherthan overwritten (so
HIDDEN_BUTTON_PROPSstill works), andtype="button"is set explicitly since MUI defaulted to it and the design-system button does
not.
Testing
tscclean; 319 suites / 3038 tests pass (the one excluded failure,invitation.integration, needs a live backend and is unrelated).auto-naming, self-relation copy and limit selection. The i18n mock resolves
against the real
en.jsonso a missing key fails the test.DatabaseTemplateButtonasserted the MUI classMuiButton-containedErrorto prove the delete confirm is destructive; now asserts the token
equivalent, same intent.
E2E run before merge, since the two updated Playwright helpers are the path
that would catch a real integration problem.
🤖 Generated with Claude Code
Summary by Sourcery
Match relation creation and editing flows to the desktop client while standardizing modal actions and related UI behavior.
New Features:
Bug Fixes:
Enhancements:
Tests:
Chores: