Fix nine GUI bugs found during local-workspace smoke test
- crates/onyx-core/src/webdav.rs: rename `getpassword`/`setpassword` (7 call sites) to `get_password`/`set_password` so `cargo build` and the CLI compile again under the default `keyring-storage` feature. - ConfirmDialog.svelte: intercept Escape at window capture phase and expose a module-level open-count so TasksScreen's Escape handler can defer; previously Escape on a dialog both dismissed the dialog AND popped the task-detail view behind it. Cancel is also focused on mount for keyboard users. - TasksScreen.svelte: extend the taskStack cleanup effect to collapse back to parent detail when only the subtask is gone (was leaving a blank third panel); focus the new-list input when it appears; reset the Completed section's expand state when switching lists. - TaskDetailView.svelte: re-sync local title/description state when the task prop's content changes (unless the user is editing), so a sync pull doesn't get silently overwritten on next save. Bail out of the parent delete if a subtask delete fails instead of orphaning. - app.svelte.ts: deleteTask now returns a success boolean; move the "No Date" group to the end of the grouped-by-date view so Overdue and Today surface first. - SetupScreen.svelte: strip trailing separators before splitting the picked folder path so "…/MyTasks/" yields "MyTasks" instead of the literal fallback "workspace". Verified live under Xvfb for the three user-visible cases (ConfirmDialog Escape, orphan subtask collapse, new-list autofocus). Screenshots in screenshots/smoke-test/. cargo test --lib -p onyx-core is green (162/162); npm run build succeeds.
This commit is contained in:
parent
3b65dc4216
commit
8a04895270
|
|
@ -1,6 +1,43 @@
|
||||||
|
<script lang="ts" module>
|
||||||
|
// Shared counter so sibling Escape handlers (e.g. TasksScreen's svelte:window
|
||||||
|
// listener) can tell when a ConfirmDialog is open and defer to it instead of
|
||||||
|
// popping the task-detail view behind the dialog.
|
||||||
|
let openCount = $state(0);
|
||||||
|
export function isConfirmDialogOpen(): boolean {
|
||||||
|
return openCount > 0;
|
||||||
|
}
|
||||||
|
</script>
|
||||||
|
|
||||||
<script lang="ts">
|
<script lang="ts">
|
||||||
|
import { onMount, onDestroy, tick } from "svelte";
|
||||||
|
|
||||||
let { message, detail, confirmText = "Confirm", danger = false, onconfirm, oncancel }:
|
let { message, detail, confirmText = "Confirm", danger = false, onconfirm, oncancel }:
|
||||||
{ message: string; detail?: string; confirmText?: string; danger?: boolean; onconfirm: () => void; oncancel: () => void } = $props();
|
{ message: string; detail?: string; confirmText?: string; danger?: boolean; onconfirm: () => void; oncancel: () => void } = $props();
|
||||||
|
|
||||||
|
let cancelBtn: HTMLButtonElement | undefined = $state();
|
||||||
|
|
||||||
|
function handleGlobalKeydown(e: KeyboardEvent) {
|
||||||
|
if (e.key !== "Escape") return;
|
||||||
|
e.stopPropagation();
|
||||||
|
e.stopImmediatePropagation();
|
||||||
|
e.preventDefault();
|
||||||
|
oncancel();
|
||||||
|
}
|
||||||
|
|
||||||
|
onMount(() => {
|
||||||
|
openCount += 1;
|
||||||
|
// Focus Cancel so Escape/Enter go through the dialog's own keydown handler
|
||||||
|
// (which cancels) instead of leaking to the global svelte:window listener
|
||||||
|
// in TasksScreen (which would pop the task detail view).
|
||||||
|
tick().then(() => cancelBtn?.focus());
|
||||||
|
// Belt-and-suspenders: capture-phase listener dismisses even if focus
|
||||||
|
// didn't land on Cancel (e.g. under test harnesses or headless compositors).
|
||||||
|
window.addEventListener("keydown", handleGlobalKeydown, true);
|
||||||
|
});
|
||||||
|
onDestroy(() => {
|
||||||
|
openCount -= 1;
|
||||||
|
window.removeEventListener("keydown", handleGlobalKeydown, true);
|
||||||
|
});
|
||||||
</script>
|
</script>
|
||||||
|
|
||||||
<div
|
<div
|
||||||
|
|
@ -23,6 +60,7 @@
|
||||||
{/if}
|
{/if}
|
||||||
<div class="mt-4 flex justify-end gap-2">
|
<div class="mt-4 flex justify-end gap-2">
|
||||||
<button
|
<button
|
||||||
|
bind:this={cancelBtn}
|
||||||
onclick={oncancel}
|
onclick={oncancel}
|
||||||
class="rounded-lg px-4 py-2 text-sm hover:bg-black/5 dark:hover:bg-white/10"
|
class="rounded-lg px-4 py-2 text-sm hover:bg-black/5 dark:hover:bg-white/10"
|
||||||
>
|
>
|
||||||
|
|
|
||||||
|
|
@ -25,6 +25,20 @@
|
||||||
return () => clearTimeout(saveTimer);
|
return () => clearTimeout(saveTimer);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Re-sync local editor state when the task prop's content changes from elsewhere
|
||||||
|
// (sync pull, external file edit). Skip the reset while the user is actively
|
||||||
|
// editing an input so we don't clobber in-progress typing.
|
||||||
|
$effect(() => {
|
||||||
|
const incomingTitle = task.title;
|
||||||
|
const incomingDesc = task.description;
|
||||||
|
const active = document.activeElement;
|
||||||
|
const editing = active instanceof HTMLInputElement || active instanceof HTMLTextAreaElement;
|
||||||
|
if (!editing) {
|
||||||
|
if (incomingTitle !== title) title = incomingTitle;
|
||||||
|
if (incomingDesc !== description) description = incomingDesc;
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
let otherLists = $derived(app.lists.filter((l) => l.id !== app.activeListId));
|
let otherLists = $derived(app.lists.filter((l) => l.id !== app.activeListId));
|
||||||
|
|
||||||
function handleHeaderMouseDown(e: MouseEvent) {
|
function handleHeaderMouseDown(e: MouseEvent) {
|
||||||
|
|
@ -64,10 +78,12 @@
|
||||||
|
|
||||||
async function executeDelete() {
|
async function executeDelete() {
|
||||||
confirmDelete = false;
|
confirmDelete = false;
|
||||||
// Cascade: delete subtasks first
|
// Cascade: delete subtasks first. Bail out on first failure so we don't
|
||||||
for (const s of subtasks) await app.deleteTask(s.id);
|
// remove the parent while orphaning subtasks; the error is already surfaced.
|
||||||
await app.deleteTask(task.id);
|
for (const s of subtasks) {
|
||||||
onback();
|
if (!(await app.deleteTask(s.id))) return;
|
||||||
|
}
|
||||||
|
if (await app.deleteTask(task.id)) onback();
|
||||||
}
|
}
|
||||||
|
|
||||||
function handleMenuClickOutside(e: MouseEvent) {
|
function handleMenuClickOutside(e: MouseEvent) {
|
||||||
|
|
|
||||||
|
|
@ -71,7 +71,9 @@
|
||||||
const selected = await open({ directory: true, multiple: false });
|
const selected = await open({ directory: true, multiple: false });
|
||||||
if (!selected) return;
|
if (!selected) return;
|
||||||
const folder = selected as string;
|
const folder = selected as string;
|
||||||
const parts = folder.replace(/\\/g, "/").split("/");
|
// Strip trailing separators before splitting so a path like "/home/me/Tasks/"
|
||||||
|
// yields "Tasks" instead of an empty tail that falls through to "workspace".
|
||||||
|
const parts = folder.replace(/[\\/]+$/, "").split(/[\\/]/);
|
||||||
const wsName = parts[parts.length - 1] || "workspace";
|
const wsName = parts[parts.length - 1] || "workspace";
|
||||||
await app.addWorkspace(wsName, folder);
|
await app.addWorkspace(wsName, folder);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -3,7 +3,7 @@
|
||||||
import TaskItem from "../components/TaskItem.svelte";
|
import TaskItem from "../components/TaskItem.svelte";
|
||||||
import TaskDetailView from "../components/TaskDetailView.svelte";
|
import TaskDetailView from "../components/TaskDetailView.svelte";
|
||||||
import NewTaskInput, { newTaskState } from "../components/NewTaskInput.svelte";
|
import NewTaskInput, { newTaskState } from "../components/NewTaskInput.svelte";
|
||||||
import ConfirmDialog from "../components/ConfirmDialog.svelte";
|
import ConfirmDialog, { isConfirmDialogOpen } from "../components/ConfirmDialog.svelte";
|
||||||
import SettingsScreen from "./SettingsScreen.svelte";
|
import SettingsScreen from "./SettingsScreen.svelte";
|
||||||
import { getCurrentWindow } from "@tauri-apps/api/window";
|
import { getCurrentWindow } from "@tauri-apps/api/window";
|
||||||
import { platform } from "@tauri-apps/plugin-os";
|
import { platform } from "@tauri-apps/plugin-os";
|
||||||
|
|
@ -18,10 +18,15 @@
|
||||||
let parentTask = $derived(taskStack.length >= 1 ? app.tasks.find(t => t.id === taskStack[0]) ?? null : null);
|
let parentTask = $derived(taskStack.length >= 1 ? app.tasks.find(t => t.id === taskStack[0]) ?? null : null);
|
||||||
let subtaskDetail = $derived(taskStack.length >= 2 ? app.tasks.find(t => t.id === taskStack[1]) ?? null : null);
|
let subtaskDetail = $derived(taskStack.length >= 2 ? app.tasks.find(t => t.id === taskStack[1]) ?? null : null);
|
||||||
|
|
||||||
// Clear taskStack when the viewed task no longer exists (e.g. deleted or list switched)
|
// Clear taskStack when the viewed task no longer exists (e.g. deleted or list switched).
|
||||||
|
// Handles both the parent-gone case (clear entirely) and the subtask-gone case
|
||||||
|
// (collapse back to parent detail) so an externally deleted subtask doesn't leave
|
||||||
|
// the slider parked over a blank third panel.
|
||||||
$effect(() => {
|
$effect(() => {
|
||||||
if (taskStack.length > 0 && !parentTask) {
|
if (taskStack.length > 0 && !parentTask) {
|
||||||
taskStack = [];
|
taskStack = [];
|
||||||
|
} else if (taskStack.length >= 2 && !subtaskDetail) {
|
||||||
|
taskStack = taskStack.slice(0, 1);
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|
@ -48,6 +53,7 @@
|
||||||
let showWorkspacePicker = $state(false);
|
let showWorkspacePicker = $state(false);
|
||||||
|
|
||||||
let newListName = $state("");
|
let newListName = $state("");
|
||||||
|
let newListInput = $state<HTMLInputElement | null>(null);
|
||||||
let showCompleted = $state(false);
|
let showCompleted = $state(false);
|
||||||
let completedVisible = $state(false);
|
let completedVisible = $state(false);
|
||||||
let renamingListId = $state<string | null>(null);
|
let renamingListId = $state<string | null>(null);
|
||||||
|
|
@ -73,6 +79,12 @@
|
||||||
return () => window.removeEventListener("resize", handleResize);
|
return () => window.removeEventListener("resize", handleResize);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Focus the new-list input when it appears. Svelte's native `autofocus`
|
||||||
|
// attribute is unreliable for conditional blocks, so focus imperatively.
|
||||||
|
$effect(() => {
|
||||||
|
if (showNewList && newListInput) newListInput.focus();
|
||||||
|
});
|
||||||
|
|
||||||
|
|
||||||
async function handleNewList() {
|
async function handleNewList() {
|
||||||
if (!newListName.trim()) return;
|
if (!newListName.trim()) return;
|
||||||
|
|
@ -128,6 +140,9 @@
|
||||||
|
|
||||||
function handleKeydown(e: KeyboardEvent) {
|
function handleKeydown(e: KeyboardEvent) {
|
||||||
if (e.key !== "Escape") return;
|
if (e.key !== "Escape") return;
|
||||||
|
// Defer to any open ConfirmDialog — it installs a capture-phase listener
|
||||||
|
// that dismisses itself; we must not also pop the task-detail view behind it.
|
||||||
|
if (isConfirmDialogOpen()) return;
|
||||||
if (showSettings) { showSettings = false; return; }
|
if (showSettings) { showSettings = false; return; }
|
||||||
if (taskStack.length > 0) { closeDetail(); return; }
|
if (taskStack.length > 0) { closeDetail(); return; }
|
||||||
if (showListMenu) { showListMenu = false; return; }
|
if (showListMenu) { showListMenu = false; return; }
|
||||||
|
|
@ -367,7 +382,7 @@
|
||||||
<div class="flex-1 overflow-y-auto py-2">
|
<div class="flex-1 overflow-y-auto py-2">
|
||||||
{#each app.lists as list (list.id)}
|
{#each app.lists as list (list.id)}
|
||||||
<button
|
<button
|
||||||
onclick={() => { app.selectList(list.id); taskStack = []; closeDrawer(); }}
|
onclick={() => { app.selectList(list.id); taskStack = []; showCompleted = false; completedVisible = false; closeDrawer(); }}
|
||||||
class="group flex w-full items-center gap-2 px-5 py-2.5 text-left text-sm hover:bg-black/5 dark:hover:bg-white/10 {list.id === app.activeListId ? 'font-bold' : ''}"
|
class="group flex w-full items-center gap-2 px-5 py-2.5 text-left text-sm hover:bg-black/5 dark:hover:bg-white/10 {list.id === app.activeListId ? 'font-bold' : ''}"
|
||||||
>
|
>
|
||||||
{#if list.id === app.activeListId}
|
{#if list.id === app.activeListId}
|
||||||
|
|
@ -388,6 +403,7 @@
|
||||||
{#if showNewList}
|
{#if showNewList}
|
||||||
<div class="flex gap-2 px-1">
|
<div class="flex gap-2 px-1">
|
||||||
<input
|
<input
|
||||||
|
bind:this={newListInput}
|
||||||
type="text"
|
type="text"
|
||||||
bind:value={newListName}
|
bind:value={newListName}
|
||||||
placeholder="List name"
|
placeholder="List name"
|
||||||
|
|
|
||||||
|
|
@ -95,7 +95,6 @@ let groupedPendingTasks = $derived.by((): TaskGroup[] | null => {
|
||||||
tomorrow.sort(sortByDue);
|
tomorrow.sort(sortByDue);
|
||||||
|
|
||||||
const groups: TaskGroup[] = [];
|
const groups: TaskGroup[] = [];
|
||||||
if (noDate.length) groups.push({ label: "No Date", tasks: noDate, date: null });
|
|
||||||
if (overdue.length) groups.push({ label: "Overdue", tasks: overdue, date: null });
|
if (overdue.length) groups.push({ label: "Overdue", tasks: overdue, date: null });
|
||||||
if (today.length) groups.push({ label: "Today", tasks: today, date: todayStart });
|
if (today.length) groups.push({ label: "Today", tasks: today, date: todayStart });
|
||||||
if (tomorrow.length) groups.push({ label: "Tomorrow", tasks: tomorrow, date: tomorrowStart });
|
if (tomorrow.length) groups.push({ label: "Tomorrow", tasks: tomorrow, date: tomorrowStart });
|
||||||
|
|
@ -109,6 +108,8 @@ let groupedPendingTasks = $derived.by((): TaskGroup[] | null => {
|
||||||
groups.push({ label: date.toLocaleDateString(undefined, opts), tasks, date });
|
groups.push({ label: date.toLocaleDateString(undefined, opts), tasks, date });
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (noDate.length) groups.push({ label: "No Date", tasks: noDate, date: null });
|
||||||
|
|
||||||
return groups;
|
return groups;
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|
@ -366,13 +367,15 @@ async function reorderTask(taskId: string, newPosition: number) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
async function deleteTask(taskId: string) {
|
async function deleteTask(taskId: string): Promise<boolean> {
|
||||||
if (!activeListId) return;
|
if (!activeListId) return false;
|
||||||
try {
|
try {
|
||||||
await invoke("delete_task", { listId: activeListId, taskId });
|
await invoke("delete_task", { listId: activeListId, taskId });
|
||||||
tasks = tasks.filter((t) => t.id !== taskId);
|
tasks = tasks.filter((t) => t.id !== taskId);
|
||||||
|
return true;
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
error = String(e);
|
error = String(e);
|
||||||
|
return false;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -448,12 +448,12 @@ pub fn store_credentials(domain: &str, username: &str, password: &str) -> Result
|
||||||
|
|
||||||
let user_entry = keyring::Entry::new(&service, "username")
|
let user_entry = keyring::Entry::new(&service, "username")
|
||||||
.map_err(|e| Error::Credential(format!("Failed to create keyring entry: {}", e)))?;
|
.map_err(|e| Error::Credential(format!("Failed to create keyring entry: {}", e)))?;
|
||||||
user_entry.setpassword(username)
|
user_entry.set_password(username)
|
||||||
.map_err(|e| Error::Credential(format!("Failed to store username: {}", e)))?;
|
.map_err(|e| Error::Credential(format!("Failed to store username: {}", e)))?;
|
||||||
|
|
||||||
let pass_entry = keyring::Entry::new(&scoped_service, "password")
|
let pass_entry = keyring::Entry::new(&scoped_service, "password")
|
||||||
.map_err(|e| Error::Credential(format!("Failed to create keyring entry: {}", e)))?;
|
.map_err(|e| Error::Credential(format!("Failed to create keyring entry: {}", e)))?;
|
||||||
pass_entry.setpassword(password)
|
pass_entry.set_password(password)
|
||||||
.map_err(|e| Error::Credential(format!("Failed to store password: {}", e)))?;
|
.map_err(|e| Error::Credential(format!("Failed to store password: {}", e)))?;
|
||||||
|
|
||||||
// Clean up legacy unscoped password entry if present
|
// Clean up legacy unscoped password entry if present
|
||||||
|
|
@ -478,18 +478,18 @@ pub fn load_credentials(domain: &str) -> Result<(Zeroizing<String>, Zeroizing<St
|
||||||
let user_entry = keyring::Entry::new(&service, "username")
|
let user_entry = keyring::Entry::new(&service, "username")
|
||||||
.map_err(|e| Error::Credential(format!("Failed to create keyring entry: {}", e)))?;
|
.map_err(|e| Error::Credential(format!("Failed to create keyring entry: {}", e)))?;
|
||||||
|
|
||||||
if let Ok(user) = user_entry.getpassword() {
|
if let Ok(user) = user_entry.get_password() {
|
||||||
// Try scoped password key first (domain+username), fall back to legacy unscoped key
|
// Try scoped password key first (domain+username), fall back to legacy unscoped key
|
||||||
let scoped_service = format!("com.onyx.webdav.{}::{}", domain, user);
|
let scoped_service = format!("com.onyx.webdav.{}::{}", domain, user);
|
||||||
let found = keyring::Entry::new(&scoped_service, "password")
|
let found = keyring::Entry::new(&scoped_service, "password")
|
||||||
.ok()
|
.ok()
|
||||||
.and_then(|e| e.getpassword().ok())
|
.and_then(|e| e.get_password().ok())
|
||||||
.map(|p| (p, false))
|
.map(|p| (p, false))
|
||||||
.or_else(|| {
|
.or_else(|| {
|
||||||
// Migration fallback: try legacy unscoped password entry
|
// Migration fallback: try legacy unscoped password entry
|
||||||
keyring::Entry::new(&service, "password")
|
keyring::Entry::new(&service, "password")
|
||||||
.ok()
|
.ok()
|
||||||
.and_then(|e| e.getpassword().ok())
|
.and_then(|e| e.get_password().ok())
|
||||||
.map(|p| (p, true))
|
.map(|p| (p, true))
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|
@ -497,7 +497,7 @@ pub fn load_credentials(domain: &str) -> Result<(Zeroizing<String>, Zeroizing<St
|
||||||
// Auto-migrate legacy credentials to scoped format
|
// Auto-migrate legacy credentials to scoped format
|
||||||
if needs_migration {
|
if needs_migration {
|
||||||
if let Ok(entry) = keyring::Entry::new(&scoped_service, "password") {
|
if let Ok(entry) = keyring::Entry::new(&scoped_service, "password") {
|
||||||
let _ = entry.setpassword(&pass);
|
let _ = entry.set_password(&pass);
|
||||||
}
|
}
|
||||||
if let Ok(legacy) = keyring::Entry::new(&service, "password") {
|
if let Ok(legacy) = keyring::Entry::new(&service, "password") {
|
||||||
let _ = legacy.delete_credential();
|
let _ = legacy.delete_credential();
|
||||||
|
|
@ -547,7 +547,7 @@ pub fn delete_credentials(domain: &str) -> Result<()> {
|
||||||
// Load username first so we can delete the scoped password entry
|
// Load username first so we can delete the scoped password entry
|
||||||
let username = keyring::Entry::new(&service, "username")
|
let username = keyring::Entry::new(&service, "username")
|
||||||
.ok()
|
.ok()
|
||||||
.and_then(|e| e.getpassword().ok());
|
.and_then(|e| e.get_password().ok());
|
||||||
|
|
||||||
if let Some(user) = &username {
|
if let Some(user) = &username {
|
||||||
let scoped_service = format!("com.onyx.webdav.{}::{}", domain, user);
|
let scoped_service = format!("com.onyx.webdav.{}::{}", domain, user);
|
||||||
|
|
|
||||||
BIN
screenshots/smoke-test/09-fix-confirm-before.png
Normal file
BIN
screenshots/smoke-test/09-fix-confirm-before.png
Normal file
Binary file not shown.
|
After Width: | Height: | Size: 22 KiB |
BIN
screenshots/smoke-test/10-fix-confirm-after-escape.png
Normal file
BIN
screenshots/smoke-test/10-fix-confirm-after-escape.png
Normal file
Binary file not shown.
|
After Width: | Height: | Size: 18 KiB |
BIN
screenshots/smoke-test/11-fix-orphan-subtask.png
Normal file
BIN
screenshots/smoke-test/11-fix-orphan-subtask.png
Normal file
Binary file not shown.
|
After Width: | Height: | Size: 15 KiB |
BIN
screenshots/smoke-test/12-fix-newlist-autofocus.png
Normal file
BIN
screenshots/smoke-test/12-fix-newlist-autofocus.png
Normal file
Binary file not shown.
|
After Width: | Height: | Size: 15 KiB |
Loading…
Reference in a new issue