fix: resolve final workbench review findings
This commit is contained in:
@@ -25,6 +25,18 @@ test('login and project channel workbench flow', async ({ page }) => {
|
||||
await expect(page.getByText('Open external channel')).toBeVisible();
|
||||
});
|
||||
|
||||
test('rejects malformed server addresses', async ({ page }) => {
|
||||
await page.goto('/');
|
||||
|
||||
await page.getByLabel('Server IP or domain').fill('https://user:secret@example.com');
|
||||
await page.getByLabel('Email or username').fill('david@example.com');
|
||||
await page.getByLabel('Password').fill('secret');
|
||||
await page.getByRole('button', { name: 'Log in' }).click();
|
||||
|
||||
await expect(page.getByText('Enter a valid HTTP or HTTPS server address.')).toBeVisible();
|
||||
await expect(page.getByLabel('Server IP or domain')).toBeVisible();
|
||||
});
|
||||
|
||||
test('mobile workbench keeps project navigation and inspector compact', async ({ page }) => {
|
||||
await page.setViewportSize({ width: 390, height: 844 });
|
||||
await page.goto('/');
|
||||
@@ -43,6 +55,12 @@ test('mobile workbench keeps project navigation and inspector compact', async ({
|
||||
await expect(rail).toHaveCSS('max-height', '76px');
|
||||
await expect(inspector).toHaveCSS('max-height', '288px');
|
||||
|
||||
const workPlan = page.getByRole('button', { name: 'Work Plan 8' });
|
||||
await expect(workPlan).toBeVisible();
|
||||
await expect(workPlan).toBeInViewport();
|
||||
await workPlan.click();
|
||||
await expect(page.getByRole('heading', { name: 'Tasks' })).toBeVisible();
|
||||
|
||||
const [sidebarBox, stageBox] = await Promise.all([sidebar.boundingBox(), stage.boundingBox()]);
|
||||
expect(sidebarBox?.y).toBeLessThan(stageBox?.y ?? 0);
|
||||
expect((stageBox?.y ?? 0) - (sidebarBox?.y ?? 0)).toBeLessThanOrEqual(248);
|
||||
|
||||
@@ -9,16 +9,19 @@
|
||||
let connectionStatus = 'Not connected. Enter a server address to continue.';
|
||||
|
||||
function normalizeServer(value: string): { apiBase?: string; error?: string } {
|
||||
const trimmed = value.trim();
|
||||
if (!trimmed) {
|
||||
if (!value.trim()) {
|
||||
return { error: 'Enter a server address.' };
|
||||
}
|
||||
|
||||
const candidate = /^https?:\/\//i.test(trimmed) ? trimmed : `http://${trimmed}`;
|
||||
if (hasWhitespace(value) || hasUnsupportedProtocol(value)) {
|
||||
return { error: 'Enter a valid HTTP or HTTPS server address.' };
|
||||
}
|
||||
|
||||
const candidate = /^https?:\/\//i.test(value) ? value : `http://${value}`;
|
||||
|
||||
try {
|
||||
const parsed = new URL(candidate);
|
||||
if (!parsed.hostname || !['http:', 'https:'].includes(parsed.protocol)) {
|
||||
if (!parsed.hostname || parsed.username || parsed.password || !['http:', 'https:'].includes(parsed.protocol)) {
|
||||
return { error: 'Enter a valid HTTP or HTTPS server address.' };
|
||||
}
|
||||
return { apiBase: candidate };
|
||||
@@ -27,6 +30,25 @@
|
||||
}
|
||||
}
|
||||
|
||||
function hasWhitespace(value: string): boolean {
|
||||
if (/\s/u.test(value)) return true;
|
||||
|
||||
try {
|
||||
return /\s/u.test(decodeURIComponent(value));
|
||||
} catch {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
function hasUnsupportedProtocol(value: string): boolean {
|
||||
const explicitProtocol = value.match(/^([a-z][a-z\d+.-]*):\/\//i);
|
||||
if (explicitProtocol) {
|
||||
return !['http', 'https'].includes(explicitProtocol[1].toLowerCase());
|
||||
}
|
||||
|
||||
return /^[a-z][a-z\d+.-]*:(?!\d)/i.test(value);
|
||||
}
|
||||
|
||||
function submitLogin() {
|
||||
error = '';
|
||||
const normalized = normalizeServer(server);
|
||||
|
||||
@@ -51,6 +51,12 @@ describe('ServerLogin', () => {
|
||||
it.each([
|
||||
['', 'Enter a server address.'],
|
||||
['http:// bad-server', 'Enter a valid HTTP or HTTPS server address.'],
|
||||
[' https://example.com', 'Enter a valid HTTP or HTTPS server address.'],
|
||||
['https://example.com/path with spaces', 'Enter a valid HTTP or HTTPS server address.'],
|
||||
['https://example%20.com', 'Enter a valid HTTP or HTTPS server address.'],
|
||||
['https://user:secret@example.com', 'Enter a valid HTTP or HTTPS server address.'],
|
||||
['http://', 'Enter a valid HTTP or HTTPS server address.'],
|
||||
['ftp://example.com', 'Enter a valid HTTP or HTTPS server address.'],
|
||||
])('rejects an invalid server value: %s', async (server, error) => {
|
||||
const onLogin = vi.fn();
|
||||
render(ServerLogin, { props: { apiBase: 'http://localhost:8080', onLogin } });
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
<script lang="ts">
|
||||
import type { InspectorItem, ProjectWorkspace, WorkbenchChannel } from './types';
|
||||
import type { InspectorItem, ProjectWorkspace, WorkTask, WorkbenchChannel } from './types';
|
||||
import OverviewChannel from './channels/OverviewChannel.svelte';
|
||||
import InboxChannel from './channels/InboxChannel.svelte';
|
||||
import TasksChannel from './channels/TasksChannel.svelte';
|
||||
@@ -9,8 +9,10 @@
|
||||
import CustomLinkChannel from './channels/CustomLinkChannel.svelte';
|
||||
|
||||
export let workspace: ProjectWorkspace;
|
||||
export let tasks: WorkTask[] = workspace.tasks;
|
||||
export let channel: WorkbenchChannel;
|
||||
export let onInspect: (item: InspectorItem) => void;
|
||||
export let onToggleTask: (taskID: string) => void = () => {};
|
||||
</script>
|
||||
|
||||
{#if channel.type === 'overview'}
|
||||
@@ -18,7 +20,7 @@
|
||||
{:else if channel.type === 'inbox'}
|
||||
<InboxChannel messages={workspace.inbox} {onInspect} />
|
||||
{:else if channel.type === 'tasks'}
|
||||
<TasksChannel tasks={workspace.tasks} {onInspect} />
|
||||
<TasksChannel {tasks} {onInspect} onToggleTask={onToggleTask} />
|
||||
{:else if channel.type === 'ai_sessions'}
|
||||
<AISessionsChannel sessions={workspace.aiSessions} {onInspect} />
|
||||
{:else if channel.type === 'notes_sources'}
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import '@testing-library/jest-dom/vitest';
|
||||
import { fireEvent, render, screen, within } from '@testing-library/svelte';
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { describe, expect, it, vi } from 'vitest';
|
||||
import ChannelContent from './ChannelContent.svelte';
|
||||
import { getProjectWorkspace } from './mockData';
|
||||
|
||||
@@ -47,18 +47,18 @@ describe('ChannelContent', () => {
|
||||
expect(inspectedTitle).toBe('Confirm homepage information architecture');
|
||||
});
|
||||
|
||||
it('toggles a task completion state accessibly', async () => {
|
||||
it('reports task completion changes to its owner', async () => {
|
||||
const channel = workspace.channels.find((item) => item.type === 'tasks');
|
||||
if (!channel) throw new Error('missing task channel');
|
||||
const onToggleTask = vi.fn();
|
||||
|
||||
render(ChannelContent, { props: { workspace, channel, onInspect: () => {} } });
|
||||
render(ChannelContent, { props: { workspace, channel, onInspect: () => {}, onToggleTask } });
|
||||
|
||||
const completion = screen.getByRole('checkbox', { name: 'Mark Confirm homepage information architecture complete' });
|
||||
expect(completion).not.toBeChecked();
|
||||
|
||||
await fireEvent.click(completion);
|
||||
|
||||
expect(completion).toBeChecked();
|
||||
expect(within(completion.closest('article') as HTMLElement).getByText('Completed')).toBeInTheDocument();
|
||||
expect(onToggleTask).toHaveBeenCalledWith('1-task-1');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -3,6 +3,19 @@
|
||||
|
||||
export let item: InspectorItem | null;
|
||||
let activeTab: 'discussion' | 'properties' | 'more' = 'discussion';
|
||||
let copyFeedback = '';
|
||||
let currentItem: InspectorItem | null = null;
|
||||
|
||||
$: if (item !== currentItem) {
|
||||
currentItem = item;
|
||||
copyFeedback = '';
|
||||
}
|
||||
|
||||
function copyLink() {
|
||||
const link = new URL(`?item=${encodeURIComponent(item?.title ?? '')}`, window.location.href).toString();
|
||||
void navigator.clipboard?.writeText(link);
|
||||
copyFeedback = 'Link copied to clipboard.';
|
||||
}
|
||||
</script>
|
||||
|
||||
<aside class="object-inspector" aria-label="Object inspector">
|
||||
@@ -27,8 +40,11 @@
|
||||
{/each}
|
||||
</dl>
|
||||
{:else}
|
||||
<button type="button">Copy link</button>
|
||||
<button type="button">Archive</button>
|
||||
<button type="button" on:click={copyLink}>Copy link</button>
|
||||
{#if copyFeedback}
|
||||
<p class="copy-feedback" role="status">{copyFeedback}</p>
|
||||
{/if}
|
||||
<button type="button" aria-label="Archive is unavailable in this preview" title="Archive is unavailable in this preview" disabled>Archive</button>
|
||||
{/if}
|
||||
{:else}
|
||||
<h2>Inspector</h2>
|
||||
|
||||
@@ -5,13 +5,17 @@
|
||||
import ProjectRail from './ProjectRail.svelte';
|
||||
import ProjectChannelSidebar from './ProjectChannelSidebar.svelte';
|
||||
import WorkspaceTopbar from './WorkspaceTopbar.svelte';
|
||||
import type { InspectorItem } from './types';
|
||||
import type { InspectorItem, WorkTask } from './types';
|
||||
|
||||
export let currentUser: { account: string };
|
||||
|
||||
let selectedProjectID = workbenchProjects[0].id;
|
||||
let selectedItem: InspectorItem | null = null;
|
||||
let tasksByProjectID: Record<number, WorkTask[]> = Object.fromEntries(
|
||||
workbenchProjects.map((project) => [project.id, getProjectWorkspace(project.id).tasks.map((task) => ({ ...task }))]),
|
||||
);
|
||||
$: workspace = getProjectWorkspace(selectedProjectID);
|
||||
$: tasks = tasksByProjectID[selectedProjectID] ?? workspace.tasks;
|
||||
$: selectedChannelID = workspace.channels[0].id;
|
||||
$: selectedChannel = workspace.channels.find((channel) => channel.id === selectedChannelID) ?? workspace.channels[0];
|
||||
|
||||
@@ -28,6 +32,13 @@
|
||||
function inspectItem(item: InspectorItem) {
|
||||
selectedItem = item;
|
||||
}
|
||||
|
||||
function toggleTask(taskID: string) {
|
||||
tasksByProjectID = {
|
||||
...tasksByProjectID,
|
||||
[selectedProjectID]: tasks.map((task) => task.id === taskID ? { ...task, completed: !task.completed } : task),
|
||||
};
|
||||
}
|
||||
</script>
|
||||
|
||||
<main class="workbench-shell">
|
||||
@@ -43,7 +54,7 @@
|
||||
onSelectChannel={selectChannel}
|
||||
/>
|
||||
<section class="channel-stage" aria-label="Channel content">
|
||||
<ChannelContent {workspace} channel={selectedChannel} onInspect={inspectItem} />
|
||||
<ChannelContent {workspace} {tasks} channel={selectedChannel} onInspect={inspectItem} onToggleTask={toggleTask} />
|
||||
</section>
|
||||
<ObjectInspector item={selectedItem} />
|
||||
</div>
|
||||
|
||||
@@ -11,7 +11,7 @@ describe('ProjectWorkbench', () => {
|
||||
expect(screen.getByRole('button', { name: 'Project A1' })).toHaveAttribute('aria-pressed', 'true');
|
||||
expect(screen.getByLabelText('Project channels')).toBeInTheDocument();
|
||||
expect(screen.getByRole('button', { name: 'Message Flow 36' })).toBeInTheDocument();
|
||||
expect(screen.getByLabelText('Global search')).toBeInTheDocument();
|
||||
expect(screen.getByLabelText('Global search')).toBeDisabled();
|
||||
expect(screen.getByLabelText('Object inspector')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
@@ -69,4 +69,34 @@ describe('ProjectWorkbench', () => {
|
||||
await fireEvent.click(screen.getByRole('button', { name: 'Message Flow 36' }));
|
||||
expect(within(inspector).getByRole('heading', { name: 'Inspector' })).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('preserves task completion when switching channels within a project', async () => {
|
||||
render(ProjectWorkbench, { props: { currentUser: { account: 'david@example.com' } } });
|
||||
|
||||
await fireEvent.click(screen.getByRole('button', { name: 'Work Plan 8' }));
|
||||
const task = screen.getByRole('checkbox', { name: 'Mark Confirm homepage information architecture complete' });
|
||||
await fireEvent.click(task);
|
||||
expect(task).toBeChecked();
|
||||
|
||||
await fireEvent.click(screen.getByRole('button', { name: 'Message Flow 36' }));
|
||||
await fireEvent.click(screen.getByRole('button', { name: 'Work Plan 8' }));
|
||||
expect(screen.getByRole('checkbox', { name: 'Mark Confirm homepage information architecture complete' })).toBeChecked();
|
||||
|
||||
await fireEvent.click(screen.getByRole('button', { name: 'Project A2' }));
|
||||
await fireEvent.click(screen.getByRole('button', { name: 'Work Plan 8' }));
|
||||
expect(screen.getByRole('checkbox', { name: 'Mark Confirm homepage information architecture complete' })).not.toBeChecked();
|
||||
});
|
||||
|
||||
it('provides Copy link feedback and disables unavailable archive actions', async () => {
|
||||
render(ProjectWorkbench, { props: { currentUser: { account: 'david@example.com' } } });
|
||||
const inspector = screen.getByLabelText('Object inspector');
|
||||
|
||||
await fireEvent.click(screen.getByRole('button', { name: 'Work Plan 8' }));
|
||||
await fireEvent.click(screen.getByRole('button', { name: 'Inspect Confirm homepage information architecture' }));
|
||||
await fireEvent.click(within(inspector).getByRole('button', { name: 'More' }));
|
||||
await fireEvent.click(within(inspector).getByRole('button', { name: 'Copy link' }));
|
||||
|
||||
expect(within(inspector).getByRole('status')).toHaveTextContent('Link copied to clipboard.');
|
||||
expect(within(inspector).getByRole('button', { name: 'Archive is unavailable in this preview' })).toBeDisabled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -6,7 +6,7 @@
|
||||
<div class="brand-mark">SenlinAI</div>
|
||||
<label class="search-box">
|
||||
<span>Search</span>
|
||||
<input aria-label="Global search" placeholder="Search projects, tasks, notes, AI sessions" />
|
||||
<input aria-label="Global search" title="Global search is unavailable in this preview" placeholder="Search projects, tasks, notes, AI sessions" disabled />
|
||||
</label>
|
||||
<div class="topbar-actions">
|
||||
<button aria-label="Back" disabled>←</button>
|
||||
|
||||
@@ -3,35 +3,25 @@
|
||||
|
||||
export let tasks: WorkTask[];
|
||||
export let onInspect: (item: InspectorItem) => void;
|
||||
|
||||
let taskStates: WorkTask[] = [];
|
||||
let sourceTasks: WorkTask[] | undefined;
|
||||
|
||||
$: if (tasks !== sourceTasks) {
|
||||
sourceTasks = tasks;
|
||||
taskStates = tasks.map((task) => ({ ...task }));
|
||||
}
|
||||
export let onToggleTask: (taskID: string) => void = () => {};
|
||||
|
||||
function inspectTask(task: WorkTask) {
|
||||
onInspect({ title: task.title, type: 'Task', description: task.summary, properties: [{ label: 'Status', value: task.completed ? 'Completed' : 'Open' }, { label: 'Owner', value: task.owner }, { label: 'Due', value: task.due }, { label: 'Tag', value: task.tag }] });
|
||||
}
|
||||
|
||||
function toggleTask(taskID: string) {
|
||||
taskStates = taskStates.map((task) => task.id === taskID ? { ...task, completed: !task.completed } : task);
|
||||
}
|
||||
</script>
|
||||
|
||||
<section class="channel-page tasks-channel">
|
||||
<header class="channel-header"><p>Work Plan</p><h1>Tasks</h1></header>
|
||||
<div class="task-list">
|
||||
{#each taskStates as task}
|
||||
{#each tasks as task}
|
||||
<article class:completed={task.completed} class="task-card">
|
||||
<input
|
||||
class="task-check"
|
||||
type="checkbox"
|
||||
aria-label={`Mark ${task.title} complete`}
|
||||
checked={task.completed}
|
||||
on:change={() => toggleTask(task.id)}
|
||||
on:change={() => onToggleTask(task.id)}
|
||||
/>
|
||||
<div>
|
||||
<h2>{task.title}</h2>
|
||||
|
||||
@@ -146,6 +146,17 @@ button:disabled { cursor: not-allowed; opacity: 0.55; }
|
||||
overflow: auto;
|
||||
}
|
||||
|
||||
.channel-sidebar {
|
||||
align-content: start;
|
||||
justify-items: start;
|
||||
}
|
||||
|
||||
.channel-sidebar > header,
|
||||
.channel-sidebar > .channel-group,
|
||||
.channel-sidebar > .recent-sessions {
|
||||
width: 100%;
|
||||
}
|
||||
|
||||
.channel-group,
|
||||
.task-list,
|
||||
.record-list,
|
||||
@@ -318,6 +329,26 @@ button:disabled { cursor: not-allowed; opacity: 0.55; }
|
||||
}
|
||||
|
||||
.channel-group button { min-width: 160px; }
|
||||
.channel-group {
|
||||
grid-auto-flow: row;
|
||||
grid-auto-columns: auto;
|
||||
grid-template-columns: repeat(3, minmax(0, 1fr));
|
||||
overflow: visible;
|
||||
}
|
||||
|
||||
.channel-group button {
|
||||
min-width: 0;
|
||||
min-height: 48px;
|
||||
grid-template-columns: 18px minmax(0, 1fr);
|
||||
padding: 8px;
|
||||
}
|
||||
|
||||
.channel-group button > span:last-of-type {
|
||||
overflow: hidden;
|
||||
text-overflow: ellipsis;
|
||||
}
|
||||
|
||||
.channel-group small { display: none; }
|
||||
.recent-sessions li { min-width: 160px; }
|
||||
.channel-stage { max-height: 36vh; overflow: auto; }
|
||||
.object-inspector { max-height: 288px; overflow: auto; }
|
||||
|
||||
Reference in New Issue
Block a user