fix: address review feedback - required field, ellipsis, edit button, tests

- Make rename input required and reject empty names in both UI and store
- Add CSS truncation with ellipsis for long entry names (max 150px)
- Move edit button inline next to name text, smaller icon
- Add rename test coverage (rename manual, auto, targeted entry)
- Fix pre-existing eslint error on external revision link (add rel=external)
This commit is contained in:
Matt Van Horn committed 2026-04-01 10:19:20 -07:00
1 parent 1d9695d82b
commit 47649f6365
3 files changed
+106 -30

No files matched your search

+32 -29
View File
@@ -174,29 +174,42 @@
<a
href={url}
target="_blank"
rel="external"
title="Open revision in new tab"
class="text-blue-500 hover:underline">{name}</a>
{:else}
{#if editingId === id}
<input
class="border rounded px-1 text-sm w-32"
bind:value={editValue}
onkeydown={(e) => {
if (e.key === 'Enter') {
renameHistoryEntry(id, editValue);
editingId = null;
}
if (e.key === 'Escape') {
editingId = null;
}
}}
onblur={() => {
renameHistoryEntry(id, editValue);
{:else if editingId === id}
<input
class="w-32 rounded border px-1 text-sm"
bind:value={editValue}
required
onkeydown={(e) => {
if (e.key === 'Enter' && editValue.trim()) {
renameHistoryEntry(id, editValue.trim());
editingId = null;
}
if (e.key === 'Escape') {
editingId = null;
}
}}
onblur={() => {
if (editValue.trim()) {
renameHistoryEntry(id, editValue.trim());
}
editingId = null;
}} />
{:else}
<span class="inline-block max-w-[150px] truncate align-middle" title={name}
>{name}</span>
{#if type !== 'loader'}
<button
class="ml-1 inline-flex items-center align-middle opacity-50 hover:opacity-100"
onclick={() => {
editingId = id;
editValue = name ?? '';
}}
/>
{:else}
<span class="whitespace-nowrap">{name}</span>
title="Rename">
<EditIcon class="h-3.5 w-3.5" />
</button>
{/if}
{/if}
<span class="text-xs whitespace-nowrap text-primary-foreground/30">
@@ -212,16 +225,6 @@
<UndoIcon />
</Button>
{#if type !== 'loader'}
<Button
size="icon"
variant="ghost"
onclick={() => {
editingId = id;
editValue = name ?? '';
}}
title="Rename">
<EditIcon />
</Button>
<Button
size="icon"
variant="ghost"
+71 -1
View File
@@ -7,7 +7,8 @@ import {
clearHistoryData,
historyModeStore,
historyStore,
injectHistoryIDs
injectHistoryIDs,
renameHistoryEntry
} from './history';
describe('history', () => {
@@ -93,6 +94,75 @@ describe('history', () => {
});
});
describe('rename history entry', () => {
it('should rename a manual history entry', () => {
historyModeStore.set('manual');
clearHistoryData();
addHistoryEntry({
state: defaultState,
time: 99_999,
type: 'manual'
});
const entries = get(historyStore);
expect(entries.length).toBe(1);
const originalName = entries[0].name;
expect(originalName).toBeDefined();
renameHistoryEntry(entries[0].id, 'my-custom-name');
expect(get(historyStore)[0].name).toBe('my-custom-name');
clearHistoryData();
});
it('should rename an auto history entry', () => {
historyModeStore.set('auto');
clearHistoryData();
addHistoryEntry({
state: defaultState,
time: 88_888,
type: 'auto'
});
const entries = get(historyStore);
expect(entries.length).toBe(1);
renameHistoryEntry(entries[0].id, 'auto-renamed');
expect(get(historyStore)[0].name).toBe('auto-renamed');
clearHistoryData();
historyModeStore.set('manual');
clearHistoryData();
});
it('should only rename the targeted entry', () => {
historyModeStore.set('manual');
clearHistoryData();
addHistoryEntry({ state: defaultState, time: 1, type: 'manual' });
addHistoryEntry({
state: { ...defaultState, code: 'graph LR\\n A --> B' },
time: 2,
type: 'manual'
});
const entries = get(historyStore);
expect(entries.length).toBe(2);
const secondId = entries[1].id;
const firstName = entries[0].name;
renameHistoryEntry(secondId, 'only-this-one');
const updated = get(historyStore);
expect(updated[0].name).toBe(firstName);
expect(updated[1].name).toBe('only-this-one');
clearHistoryData();
});
});
describe('history migration', () => {
it('should inject history IDs as migration', () => {
window.localStorage.setItem(
+3
View File
@@ -90,6 +90,9 @@ export const clearHistoryData = (idToClear?: string): void => {
};
export const renameHistoryEntry = (idToRename: string, newName: string): void => {
if (!newName.trim()) {
return;
}
(get(historyModeStore) === 'auto' ? autoHistoryStore : manualHistoryStore).update((entries) => {
return entries.map((entry) => (entry.id === idToRename ? { ...entry, name: newName } : entry));
});