Rough edges: mobile 100dvh, session-expiry in chat, sticky error toasts, journal date filter #93
@@ -17,9 +17,16 @@ interface ToastState {
|
|||||||
|
|
||||||
let nextId = 1
|
let nextId = 1
|
||||||
|
|
||||||
|
// Error toasts no longer auto-dismiss (#85), so a burst of failures could grow
|
||||||
|
// the stack without bound and push older ones off-screen. Cap it: keep the most
|
||||||
|
// recent MAX_TOASTS and drop the oldest, so the newest — the one that just
|
||||||
|
// happened — is always visible.
|
||||||
|
const MAX_TOASTS = 4
|
||||||
|
|
||||||
export const useToastStore = create<ToastState>((set) => ({
|
export const useToastStore = create<ToastState>((set) => ({
|
||||||
toasts: [],
|
toasts: [],
|
||||||
push: (message, tone = 'info') => set((s) => ({ toasts: [...s.toasts, { id: nextId++, message, tone }] })),
|
push: (message, tone = 'info') =>
|
||||||
|
set((s) => ({ toasts: [...s.toasts, { id: nextId++, message, tone }].slice(-MAX_TOASTS) })),
|
||||||
dismiss: (id) => set((s) => ({ toasts: s.toasts.filter((t) => t.id !== id) })),
|
dismiss: (id) => set((s) => ({ toasts: s.toasts.filter((t) => t.id !== id) })),
|
||||||
}))
|
}))
|
||||||
|
|
||||||
@@ -32,21 +39,34 @@ export const toast = {
|
|||||||
// Param is `item`, not `toast`, so it doesn't shadow the module's `toast` export.
|
// Param is `item`, not `toast`, so it doesn't shadow the module's `toast` export.
|
||||||
function ToastItem({ item }: { item: Toast }) {
|
function ToastItem({ item }: { item: Toast }) {
|
||||||
|
|
|||||||
const dismiss = useToastStore((s) => s.dismiss)
|
const dismiss = useToastStore((s) => s.dismiss)
|
||||||
|
const isError = item.tone === 'error'
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
|
// Error toasts are the primary report that a mutation failed, so they do NOT
|
||||||
|
// auto-dismiss — a user who looked away at second 4 would otherwise lose the
|
||||||
|
// only notice, with nothing to retrieve (#85). Info toasts still time out.
|
||||||
|
if (isError) return
|
||||||
const t = setTimeout(() => dismiss(item.id), 4000)
|
const t = setTimeout(() => dismiss(item.id), 4000)
|
||||||
return () => clearTimeout(t)
|
return () => clearTimeout(t)
|
||||||
}, [item.id, dismiss])
|
}, [item.id, dismiss, isError])
|
||||||
return (
|
return (
|
||||||
<div
|
<div
|
||||||
role={item.tone === 'error' ? 'alert' : 'status'}
|
role={isError ? 'alert' : 'status'}
|
||||||
className={cn(
|
className={cn(
|
||||||
'pointer-events-auto rounded-md border px-3 py-2 text-sm shadow-md',
|
'pointer-events-auto flex items-start gap-2 rounded-md border px-3 py-2 text-sm shadow-md',
|
||||||
item.tone === 'error'
|
isError
|
||||||
? 'border-red-500/40 bg-red-500/10 text-red-700 dark:text-red-300'
|
? 'border-red-500/40 bg-red-500/10 text-red-700 dark:text-red-300'
|
||||||
: 'border-border bg-surface text-fg',
|
: 'border-border bg-surface text-fg',
|
||||||
)}
|
)}
|
||||||
>
|
>
|
||||||
{item.message}
|
<span className="flex-1">{item.message}</span>
|
||||||
|
<button
|
||||||
|
type="button"
|
||||||
|
onClick={() => dismiss(item.id)}
|
||||||
|
aria-label="Dismiss"
|
||||||
|
className="-mr-1 shrink-0 rounded px-1 text-current opacity-60 outline-none hover:opacity-100 focus-visible:ring-2 focus-visible:ring-current/40"
|
||||||
|
>
|
||||||
|
✕
|
||||||
|
</button>
|
||||||
</div>
|
</div>
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -16,6 +16,11 @@ import {
|
|||||||
import type { EditorObject } from './types'
|
import type { EditorObject } from './types'
|
||||||
import { objectDisplayName } from './kinds'
|
import { objectDisplayName } from './kinds'
|
||||||
|
|
||||||
|
// Shared styling for the small From/To date inputs, so the two stay in step and
|
||||||
|
// don't drift from each other.
|
||||||
|
const dateInputClass =
|
||||||
|
'rounded-md border border-border bg-surface px-1.5 py-1 text-fg outline-none focus-visible:ring-2 focus-visible:ring-accent/40'
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* The garden's journal: write an entry, read the season back.
|
* The garden's journal: write an entry, read the season back.
|
||||||
*
|
*
|
||||||
@@ -42,7 +47,15 @@ export function JournalPanel({
|
|||||||
scopeObjectId: number | null
|
scopeObjectId: number | null
|
||||||
onScopeChange: (id: number | null) => void
|
onScopeChange: (id: number | null) => void
|
||||||
}) {
|
}) {
|
||||||
|
gitea-actions
commented
⚪ filter built from three stacked ternary-spread branches; a compact helper or plain assignments would scale better as filter fields grow maintainability · flagged by 1 model
🪰 Gadfly · advisory ⚪ **filter built from three stacked ternary-spread branches; a compact helper or plain assignments would scale better as filter fields grow**
_maintainability · flagged by 1 model_
- **`web/src/editor/JournalPanel.tsx:49-53` — `filter` built from three stacked ternary-spread branches.** The `...(scopeObjectId != null ? { objectId: scopeObjectId } : {})` idiom, stacked three times, reads as clever rather than clear. A `compact`/`Object.fromEntries(Object.entries({...}).filter(...))` helper or plain assignments into a `const filter: JournalFilter = {}` would scale better as more filter fields appear. Trivial maintainability nit.
<sub>🪰 Gadfly · advisory</sub>
|
|||||||
const filter = scopeObjectId != null ? { objectId: scopeObjectId } : {}
|
// Date-range narrowing (#85): the backend and JournalFilter already supported
|
||||||
|
// from/to; they just had no UI. Empty inputs don't filter.
|
||||||
|
const [from, setFrom] = useState('')
|
||||||
|
const [to, setTo] = useState('')
|
||||||
|
const filter = {
|
||||||
|
...(scopeObjectId != null ? { objectId: scopeObjectId } : {}),
|
||||||
|
...(from ? { from } : {}),
|
||||||
|
...(to ? { to } : {}),
|
||||||
|
}
|
||||||
const journal = useJournal(gardenId, filter)
|
const journal = useJournal(gardenId, filter)
|
||||||
const entries = journal.data?.pages.flatMap((p) => p.entries) ?? []
|
const entries = journal.data?.pages.flatMap((p) => p.entries) ?? []
|
||||||
const scopedObject = objects.find((o) => o.id === scopeObjectId) ?? null
|
const scopedObject = objects.find((o) => o.id === scopeObjectId) ?? null
|
||||||
@@ -70,6 +83,41 @@ export function JournalPanel({
|
|||||||
)}
|
)}
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
|
<div className="flex items-center gap-2 text-xs text-muted">
|
||||||
|
<label className="flex items-center gap-1">
|
||||||
|
<span>From</span>
|
||||||
|
<input
|
||||||
|
gitea-actions
commented
🟡 Journal date filter inputs hand-roll styling instead of reusing shared fieldControlClass, dropping text-base (iOS zoom-on-focus) and diverging focus-ring styling from every other input in the app maintainability · flagged by 2 models
🪰 Gadfly · advisory 🟡 **Journal date filter inputs hand-roll styling instead of reusing shared fieldControlClass, dropping text-base (iOS zoom-on-focus) and diverging focus-ring styling from every other input in the app**
_maintainability · flagged by 2 models_
- **`web/src/editor/JournalPanel.tsx:89,99` — date-input control styling duplicated verbatim with no shared class constant.** The `rounded-md border border-border bg-surface px-1.5 py-1 text-fg outline-none focus-visible:ring-2 focus-visible:ring-accent/40` string is repeated identically for the `From` and `To` inputs (lines 89 and 99), and a near-identical variant already exists for the scope `<select>` at line 69 and the `Clear` button at line 109. A small `const inputCls = '…'` at module top…
<sub>🪰 Gadfly · advisory</sub>
|
|||||||
|
type="date"
|
||||||
|
value={from}
|
||||||
|
max={to || undefined}
|
||||||
|
onChange={(e) => setFrom(e.target.value)}
|
||||||
|
className={dateInputClass}
|
||||||
|
/>
|
||||||
|
</label>
|
||||||
|
<label className="flex items-center gap-1">
|
||||||
|
<span>To</span>
|
||||||
|
<input
|
||||||
|
type="date"
|
||||||
|
value={to}
|
||||||
|
min={from || undefined}
|
||||||
|
onChange={(e) => setTo(e.target.value)}
|
||||||
|
className={dateInputClass}
|
||||||
|
/>
|
||||||
|
</label>
|
||||||
|
{(from || to) && (
|
||||||
|
<button
|
||||||
|
type="button"
|
||||||
|
onClick={() => {
|
||||||
|
setFrom('')
|
||||||
|
setTo('')
|
||||||
|
}}
|
||||||
|
className="rounded px-1 text-muted outline-none hover:text-fg focus-visible:ring-2 focus-visible:ring-accent/40"
|
||||||
|
>
|
||||||
|
Clear
|
||||||
|
</button>
|
||||||
|
)}
|
||||||
|
</div>
|
||||||
|
|
||||||
{canEdit && (
|
{canEdit && (
|
||||||
<Composer
|
<Composer
|
||||||
gardenId={gardenId}
|
gardenId={gardenId}
|
||||||
|
|||||||
@@ -160,6 +160,15 @@ export async function streamChat(
|
|||||||
handlers.onError('Could not reach the server.')
|
handlers.onError('Could not reach the server.')
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
if (res.status === 401) {
|
||||||
|
// Session expired mid-conversation (#85). Reporting this as "the assistant is
|
||||||
|
// unavailable" would send the user chasing a config problem that isn't there.
|
||||||
|
// Send them to sign in again, preserving where they were.
|
||||||
|
handlers.onError('Your session has expired — please sign in again.')
|
||||||
|
const back = encodeURIComponent(location.pathname + location.search)
|
||||||
|
gitea-actions
commented
🟡 401 redirect hand-rolls encodeURIComponent + window.location.assign instead of reusing safeRedirectPath for the value construction used elsewhere for auth redirects maintainability · flagged by 1 model
🪰 Gadfly · advisory 🟡 **401 redirect hand-rolls encodeURIComponent + window.location.assign instead of reusing safeRedirectPath for the value construction used elsewhere for auth redirects**
_maintainability · flagged by 1 model_
- **`web/src/lib/agent.ts:168-169` — 401 redirect hand-rolls the login redirect instead of routing through the codebase's centralized `safeRedirectPath`/router pattern.** The rest of the app funnels auth redirects through `safeRedirectPath` (`web/src/lib/redirect.ts:12`, used at `web/src/router.tsx:40,59,90` and `web/src/pages/LoginPage.tsx:36`), whose header comment explicitly states "both go through this so a caller-controlled value can never send the user to an external origin." The new 401 p…
<sub>🪰 Gadfly · advisory</sub>
|
|||||||
|
window.location.assign(`/login?redirect=${back}`)
|
||||||
|
return
|
||||||
|
}
|
||||||
if (!res.ok || !res.body) {
|
if (!res.ok || !res.body) {
|
||||||
// 503 is the assistant being turned off at runtime (#79) — the route exists,
|
// 503 is the assistant being turned off at runtime (#79) — the route exists,
|
||||||
// there's just no model behind it. Distinct from a 404, which would mean the
|
// there's just no model behind it. Distinct from a 404, which would mean the
|
||||||
|
|||||||
@@ -398,8 +398,11 @@ export function GardenEditorPage() {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// 100dvh, not 100vh: on mobile Safari/Chrome 100vh is the *largest* viewport
|
||||||
|
// (URL bar hidden), so with the bar showing the editor overflowed and pushed
|
||||||
|
// the canvas bottom + Fit button under the browser chrome (#85).
|
||||||
return (
|
return (
|
||||||
<div className="flex h-[calc(100vh-8rem)] flex-col gap-3 md:flex-row">
|
<div className="flex h-[calc(100dvh-8rem)] flex-col gap-3 md:flex-row">
|
||||||
<div className="shrink-0 md:w-40">
|
<div className="shrink-0 md:w-40">
|
||||||
<h1 className="mb-2 truncate text-lg font-semibold tracking-tight" title={garden.name}>
|
<h1 className="mb-2 truncate text-lg font-semibold tracking-tight" title={garden.name}>
|
||||||
{garden.name}
|
{garden.name}
|
||||||
|
|||||||
@@ -53,7 +53,7 @@ export function PublicGardenPage() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<div className="flex h-[calc(100vh-8rem)] flex-col gap-3">
|
<div className="flex h-[calc(100dvh-8rem)] flex-col gap-3">
|
||||||
<div className="flex flex-wrap items-center gap-2">
|
<div className="flex flex-wrap items-center gap-2">
|
||||||
<h1 className="truncate text-lg font-semibold tracking-tight" title={garden.name}>
|
<h1 className="truncate text-lg font-semibold tracking-tight" title={garden.name}>
|
||||||
{garden.name}
|
{garden.name}
|
||||||
|
|||||||
Reference in New Issue
Block a user
🟡 Error toasts no longer auto-dismiss and the store has no cap, so repeated errors grow the toast stack/DOM unbounded
performance · flagged by 1 model
web/src/components/ui/toast.tsx:40— error toasts now accumulate with no bound. Previously every toast auto-dismissed at 4s, so thetoastsarray in the zustand store self-drained. Now error toasts never time out and are removed only by an explicitdismissclick. The store has no max-cap —pushjust appends (line 22:set((s) => ({ toasts: [...s.toasts, { id: nextId++, message, tone }] }))) — andToasterrenders the entire array (lines 73-75). Any path that fires repeated `toast.…🪰 Gadfly · advisory