fix: relation chips now show type color instead of default blue (#68)
* fix: relation chips now display correct type color instead of default blue Root cause: resolveRef matched entries by path/filename only, while wiki-links used findEntryByTarget (title, filename, aliases). When a wikilink target used the note title (differing from filename), resolveRef failed → null type → default blue. Changes: - resolveRef now calls findEntryByTarget first (title/alias match) - Default color for typeless notes changed from blue to neutral grey - TypeSelector passes custom color key from Type entries - DynamicPropertiesPanel refactored to reduce cyclomatic complexity Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * design: add before/after visual for relations type color fix Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs: add task completion summary Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
3
.claude-done
Normal file
3
.claude-done
Normal file
@@ -0,0 +1,3 @@
|
||||
Task: relations-type-color
|
||||
Summary: Fixed relation chips showing default blue instead of type color. Root cause: resolveRef matched entries by path/filename only, while wiki-links used findEntryByTarget (title, filename, aliases). Fixed by adding title/alias matching to resolveRef, changing default color from blue to neutral grey for typeless notes, and passing custom color keys to TypeSelector. Also refactored DynamicPropertiesPanel to reduce cyclomatic complexity (11→10). All tests pass (739), code health gates pass, coverage 79%.
|
||||
Commits: 2
|
||||
1
design/relations-type-color.pen
Normal file
1
design/relations-type-color.pen
Normal file
@@ -0,0 +1 @@
|
||||
{"children":[],"variables":{}}
|
||||
@@ -147,7 +147,7 @@ function AddPropertyForm({ onAdd, onCancel }: { onAdd: (key: string, value: stri
|
||||
|
||||
const TYPE_NONE = '__none__'
|
||||
|
||||
function ReadOnlyType({ isA, onNavigate }: { isA?: string | null; onNavigate?: (target: string) => void }) {
|
||||
function ReadOnlyType({ isA, customColorKey, onNavigate }: { isA?: string | null; customColorKey?: string | null; onNavigate?: (target: string) => void }) {
|
||||
if (!isA) return null
|
||||
return (
|
||||
<div className="flex items-center justify-between px-1.5">
|
||||
@@ -155,7 +155,7 @@ function ReadOnlyType({ isA, onNavigate }: { isA?: string | null; onNavigate?: (
|
||||
{onNavigate ? (
|
||||
<button
|
||||
className="min-w-0 truncate border-none text-right cursor-pointer hover:opacity-80"
|
||||
style={{ background: getTypeLightColor(isA), color: getTypeColor(isA), borderRadius: 6, padding: '2px 8px', fontSize: 12, fontWeight: 500 }}
|
||||
style={{ background: getTypeLightColor(isA, customColorKey), color: getTypeColor(isA, customColorKey), borderRadius: 6, padding: '2px 8px', fontSize: 12, fontWeight: 500 }}
|
||||
onClick={() => onNavigate(`type/${isA.toLowerCase()}`)} title={isA}
|
||||
>{isA}</button>
|
||||
) : (
|
||||
@@ -165,12 +165,12 @@ function ReadOnlyType({ isA, onNavigate }: { isA?: string | null; onNavigate?: (
|
||||
)
|
||||
}
|
||||
|
||||
function TypeSelector({ isA, availableTypes, onUpdateProperty, onNavigate }: {
|
||||
isA?: string | null; availableTypes: string[]
|
||||
function TypeSelector({ isA, customColorKey, availableTypes, onUpdateProperty, onNavigate }: {
|
||||
isA?: string | null; customColorKey?: string | null; availableTypes: string[]
|
||||
onUpdateProperty?: (key: string, value: FrontmatterValue) => void
|
||||
onNavigate?: (target: string) => void
|
||||
}) {
|
||||
if (!onUpdateProperty) return <ReadOnlyType isA={isA} onNavigate={onNavigate} />
|
||||
if (!onUpdateProperty) return <ReadOnlyType isA={isA} customColorKey={customColorKey} onNavigate={onNavigate} />
|
||||
|
||||
const currentValue = isA || TYPE_NONE
|
||||
const options = isA && !availableTypes.includes(isA)
|
||||
@@ -185,8 +185,8 @@ function TypeSelector({ isA, availableTypes, onUpdateProperty, onNavigate }: {
|
||||
size="sm"
|
||||
className="h-auto min-h-0 gap-1 border-none px-2 py-0.5 shadow-none"
|
||||
style={isA ? {
|
||||
background: getTypeLightColor(isA),
|
||||
color: getTypeColor(isA),
|
||||
background: getTypeLightColor(isA, customColorKey),
|
||||
color: getTypeColor(isA, customColorKey),
|
||||
fontSize: 12,
|
||||
fontWeight: 500,
|
||||
borderRadius: 6,
|
||||
@@ -259,6 +259,31 @@ function AddPropertyButton({ onClick, disabled }: { onClick: () => void; disable
|
||||
)
|
||||
}
|
||||
|
||||
function NoteInfoSection({ entry, wordCount }: { entry: VaultEntry; wordCount: number }) {
|
||||
return (
|
||||
<div className="border-t border-border pt-3">
|
||||
<h4 className="font-mono-overline mb-2 text-muted-foreground">Info</h4>
|
||||
<div className="flex flex-col gap-1.5">
|
||||
<InfoRow label="Modified" value={formatDate(entry.modifiedAt)} />
|
||||
<InfoRow label="Created" value={formatDate(entry.createdAt)} />
|
||||
<InfoRow label="Words" value={String(wordCount)} />
|
||||
<InfoRow label="Size" value={formatFileSize(entry.fileSize)} />
|
||||
</div>
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
||||
function reconcileListUpdate(
|
||||
newItems: string[],
|
||||
onUpdate: (key: string, value: FrontmatterValue) => void,
|
||||
onDelete: ((key: string) => void) | undefined,
|
||||
key: string,
|
||||
) {
|
||||
if (newItems.length === 0) onDelete?.(key)
|
||||
else if (newItems.length === 1) onUpdate(key, newItems[0])
|
||||
else onUpdate(key, newItems)
|
||||
}
|
||||
|
||||
export function DynamicPropertiesPanel({
|
||||
entry,
|
||||
content,
|
||||
@@ -283,12 +308,13 @@ export function DynamicPropertiesPanel({
|
||||
|
||||
const wordCount = countWords(content ?? '')
|
||||
|
||||
const availableTypes = useMemo(() =>
|
||||
(entries ?? [])
|
||||
.filter(e => e.isA === 'Type')
|
||||
.map(e => e.title)
|
||||
.sort((a, b) => a.localeCompare(b))
|
||||
, [entries])
|
||||
const { availableTypes, customColorKey } = useMemo(() => {
|
||||
const typeEntries = (entries ?? []).filter(e => e.isA === 'Type')
|
||||
return {
|
||||
availableTypes: typeEntries.map(e => e.title).sort((a, b) => a.localeCompare(b)),
|
||||
customColorKey: entry.isA ? (typeEntries.find(e => e.title === entry.isA)?.color ?? null) : null,
|
||||
}
|
||||
}, [entries, entry.isA])
|
||||
|
||||
const propertyEntries = useMemo(() => {
|
||||
return Object.entries(frontmatter)
|
||||
@@ -302,9 +328,7 @@ export function DynamicPropertiesPanel({
|
||||
|
||||
const handleSaveList = useCallback((key: string, newItems: string[]) => {
|
||||
if (!onUpdateProperty) return
|
||||
if (newItems.length === 0) onDeleteProperty?.(key)
|
||||
else if (newItems.length === 1) onUpdateProperty(key, newItems[0])
|
||||
else onUpdateProperty(key, newItems)
|
||||
reconcileListUpdate(newItems, onUpdateProperty, onDeleteProperty, key)
|
||||
}, [onUpdateProperty, onDeleteProperty])
|
||||
|
||||
const handleAdd = useCallback((rawKey: string, rawValue: string) => {
|
||||
@@ -317,7 +341,7 @@ export function DynamicPropertiesPanel({
|
||||
<div className="flex flex-col gap-3">
|
||||
{/* Editable properties section */}
|
||||
<div className="flex flex-col gap-2">
|
||||
<TypeSelector isA={entry.isA} availableTypes={availableTypes} onUpdateProperty={onUpdateProperty} onNavigate={onNavigate} />
|
||||
<TypeSelector isA={entry.isA} customColorKey={customColorKey} availableTypes={availableTypes} onUpdateProperty={onUpdateProperty} onNavigate={onNavigate} />
|
||||
{propertyEntries.map(([key, value]) => (
|
||||
<PropertyRow key={key} propKey={key} value={value} editingKey={editingKey} onStartEdit={setEditingKey} onSave={handleSaveValue} onSaveList={handleSaveList} onUpdate={onUpdateProperty} onDelete={onDeleteProperty} />
|
||||
))}
|
||||
@@ -326,16 +350,7 @@ export function DynamicPropertiesPanel({
|
||||
? <AddPropertyForm onAdd={handleAdd} onCancel={() => setShowAddDialog(false)} />
|
||||
: <AddPropertyButton onClick={() => setShowAddDialog(true)} disabled={!onAddProperty} />
|
||||
}
|
||||
{/* Read-only Info section */}
|
||||
<div className="border-t border-border pt-3">
|
||||
<h4 className="font-mono-overline mb-2 text-muted-foreground">Info</h4>
|
||||
<div className="flex flex-col gap-1.5">
|
||||
<InfoRow label="Modified" value={formatDate(entry.modifiedAt)} />
|
||||
<InfoRow label="Created" value={formatDate(entry.createdAt)} />
|
||||
<InfoRow label="Words" value={String(wordCount)} />
|
||||
<InfoRow label="Size" value={formatFileSize(entry.fileSize)} />
|
||||
</div>
|
||||
</div>
|
||||
<NoteInfoSection entry={entry} wordCount={wordCount} />
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
||||
@@ -32,15 +32,51 @@ const makeEntry = (overrides: Partial<VaultEntry> = {}): VaultEntry => ({
|
||||
describe('DynamicRelationshipsPanel', () => {
|
||||
const onNavigate = vi.fn()
|
||||
const onAddProperty = vi.fn()
|
||||
const personTypeEntry = makeEntry({
|
||||
path: '/vault/type/person.md', filename: 'person.md', title: 'Person',
|
||||
isA: 'Type', color: 'yellow', icon: 'user',
|
||||
})
|
||||
const entries = [
|
||||
makeEntry({ path: '/vault/project/my-project.md', filename: 'my-project.md', title: 'My Project', isA: 'Project' }),
|
||||
makeEntry({ path: '/vault/topic/ai.md', filename: 'ai.md', title: 'AI', isA: 'Topic' }),
|
||||
personTypeEntry,
|
||||
]
|
||||
const typeEntryMap: Record<string, VaultEntry> = { Person: personTypeEntry }
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
})
|
||||
|
||||
it.each([
|
||||
{
|
||||
name: 'applies type color via typeEntryMap',
|
||||
entry: { path: '/vault/people/luca.md', filename: 'luca.md', title: 'Luca', isA: 'Person' as const },
|
||||
ref: '[[Luca]]', key: 'Owner', tMap: typeEntryMap, expectedColor: 'var(--accent-yellow)',
|
||||
},
|
||||
{
|
||||
name: 'resolves by title when filename differs',
|
||||
entry: { path: '/vault/people/john-doe.md', filename: 'john-doe.md', title: 'John Doe', isA: 'Person' as const },
|
||||
ref: '[[John Doe]]', key: 'Owner', tMap: typeEntryMap, expectedColor: 'var(--accent-yellow)',
|
||||
},
|
||||
{
|
||||
name: 'shows neutral color for notes with no type',
|
||||
entry: { path: '/vault/misc/random.md', filename: 'random.md', title: 'Random', isA: null },
|
||||
ref: '[[Random]]', key: 'Related to', tMap: {} as Record<string, VaultEntry>, expectedColor: 'var(--muted-foreground)',
|
||||
},
|
||||
])('$name', ({ entry: overrides, ref, key, tMap, expectedColor }) => {
|
||||
const { container } = render(
|
||||
<DynamicRelationshipsPanel
|
||||
typeEntryMap={tMap}
|
||||
frontmatter={{ [key]: [ref] }}
|
||||
entries={[...entries, makeEntry(overrides)]}
|
||||
onNavigate={onNavigate}
|
||||
/>
|
||||
)
|
||||
const chip = container.querySelector('.group\\/link button')
|
||||
expect(chip).toBeTruthy()
|
||||
expect(chip!.style.color).toBe(expectedColor)
|
||||
})
|
||||
|
||||
it('shows "No relationships" when frontmatter has no relations', () => {
|
||||
render(
|
||||
<DynamicRelationshipsPanel
|
||||
|
||||
@@ -7,6 +7,7 @@ import type { ParsedFrontmatter } from '../utils/frontmatter'
|
||||
import { RELATIONSHIP_KEYS, containsWikilinks } from './DynamicPropertiesPanel'
|
||||
import { getTypeColor, getTypeLightColor } from '../utils/typeColors'
|
||||
import { getTypeIcon } from './NoteItem'
|
||||
import { findEntryByTarget } from '../utils/wikilinkColors'
|
||||
import type { FrontmatterValue } from './Inspector'
|
||||
|
||||
function isWikilink(value: string): boolean {
|
||||
@@ -15,10 +16,13 @@ function isWikilink(value: string): boolean {
|
||||
|
||||
function resolveRef(ref: string, entries: VaultEntry[]): VaultEntry | undefined {
|
||||
const target = wikilinkTarget(ref)
|
||||
const byTitle = findEntryByTarget(entries, target)
|
||||
if (byTitle) return byTitle
|
||||
const lastSegment = target.split('/').pop()
|
||||
return entries.find((e) => {
|
||||
const stem = e.path.replace(/^.*\/Laputa\//, '').replace(/\.md$/, '')
|
||||
if (stem === target) return true
|
||||
return e.filename.replace(/\.md$/, '') === target.split('/').pop()
|
||||
return e.filename.replace(/\.md$/, '') === lastSegment
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
49
src/utils/typeColors.test.ts
Normal file
49
src/utils/typeColors.test.ts
Normal file
@@ -0,0 +1,49 @@
|
||||
import { describe, it, expect } from 'vitest'
|
||||
import { getTypeColor, getTypeLightColor } from './typeColors'
|
||||
|
||||
describe('getTypeColor', () => {
|
||||
it('returns hardcoded color for known types', () => {
|
||||
expect(getTypeColor('Project')).toBe('var(--accent-red)')
|
||||
expect(getTypeColor('Person')).toBe('var(--accent-yellow)')
|
||||
expect(getTypeColor('Topic')).toBe('var(--accent-green)')
|
||||
})
|
||||
|
||||
it('returns neutral muted color for null type', () => {
|
||||
expect(getTypeColor(null)).toBe('var(--muted-foreground)')
|
||||
})
|
||||
|
||||
it('returns neutral muted color for unknown type without custom key', () => {
|
||||
expect(getTypeColor('UnknownType')).toBe('var(--muted-foreground)')
|
||||
})
|
||||
|
||||
it('uses custom color key over hardcoded map', () => {
|
||||
expect(getTypeColor('Project', 'green')).toBe('var(--accent-green)')
|
||||
})
|
||||
|
||||
it('uses custom color key for unknown type', () => {
|
||||
expect(getTypeColor('Recipe', 'orange')).toBe('var(--accent-orange)')
|
||||
})
|
||||
|
||||
it('ignores invalid custom color key', () => {
|
||||
expect(getTypeColor('Project', 'invalid')).toBe('var(--accent-red)')
|
||||
})
|
||||
})
|
||||
|
||||
describe('getTypeLightColor', () => {
|
||||
it('returns hardcoded light color for known types', () => {
|
||||
expect(getTypeLightColor('Project')).toBe('var(--accent-red-light)')
|
||||
expect(getTypeLightColor('Person')).toBe('var(--accent-yellow-light)')
|
||||
})
|
||||
|
||||
it('returns neutral muted light color for null type', () => {
|
||||
expect(getTypeLightColor(null)).toBe('var(--muted)')
|
||||
})
|
||||
|
||||
it('returns neutral muted light color for unknown type without custom key', () => {
|
||||
expect(getTypeLightColor('UnknownType')).toBe('var(--muted)')
|
||||
})
|
||||
|
||||
it('uses custom color key for light variant', () => {
|
||||
expect(getTypeLightColor('Recipe', 'purple')).toBe('var(--accent-purple-light)')
|
||||
})
|
||||
})
|
||||
@@ -25,8 +25,8 @@ const TYPE_LIGHT_COLOR_MAP: Record<string, string> = {
|
||||
Type: 'var(--accent-blue-light)',
|
||||
}
|
||||
|
||||
const DEFAULT_COLOR = 'var(--accent-blue)'
|
||||
const DEFAULT_LIGHT_COLOR = 'var(--accent-blue-light)'
|
||||
const DEFAULT_COLOR = 'var(--muted-foreground)'
|
||||
const DEFAULT_LIGHT_COLOR = 'var(--muted)'
|
||||
|
||||
/** Color key → CSS variable mapping for the design system accent palette */
|
||||
export const ACCENT_COLORS: { key: string; label: string; css: string; cssLight: string }[] = [
|
||||
|
||||
@@ -87,8 +87,8 @@ describe('lookupColorForEntry', () => {
|
||||
expect(lookupColorForEntry(allEntries, recipeEntry)).toBe('var(--accent-orange)')
|
||||
})
|
||||
|
||||
it('returns default blue for an entry with no type', () => {
|
||||
expect(lookupColorForEntry(allEntries, untypedEntry)).toBe('var(--accent-blue)')
|
||||
it('returns neutral color for an entry with no type', () => {
|
||||
expect(lookupColorForEntry(allEntries, untypedEntry)).toBe('var(--muted-foreground)')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -105,16 +105,16 @@ describe('resolveWikilinkColor', () => {
|
||||
expect(result.color).toBe('var(--text-muted)')
|
||||
})
|
||||
|
||||
it('returns default color for an untyped note', () => {
|
||||
it('returns neutral color for an untyped note', () => {
|
||||
const result = resolveWikilinkColor(allEntries, 'Random Thought')
|
||||
expect(result.isBroken).toBe(false)
|
||||
expect(result.color).toBe('var(--accent-blue)')
|
||||
expect(result.color).toBe('var(--muted-foreground)')
|
||||
})
|
||||
|
||||
it('returns default color when entries list is empty', () => {
|
||||
it('returns neutral color when entries list is empty', () => {
|
||||
const result = resolveWikilinkColor([], 'Anything')
|
||||
expect(result.isBroken).toBe(false)
|
||||
expect(result.color).toBe('var(--accent-blue)')
|
||||
expect(result.color).toBe('var(--muted-foreground)')
|
||||
})
|
||||
|
||||
it('resolves alias-based wikilink target', () => {
|
||||
|
||||
Reference in New Issue
Block a user