Move log sort selector from Log Preview to Timeline header
The "Sort by" / "fall back to ottyl_seq" control now lives in LogTimeline's header next to the time-range presets, since the setting actually drives the shared range/log fetch rather than being a Log-Preview-only concern. LogSortSettings and its load/save helpers moved into LogTimeline.tsx alongside SortPicker; the localStorage key was renamed to ottyl.logTimeline.sort, with loadLogSortSettings falling back to the old ottyl.logPreview.sort key so an existing choice isn't lost on first load after the move. LogPreview now just renders logs in whatever order the caller already sorted them. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XFUNZwCvz3kcod3352ZR2o
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
d17cfe9b83
commit
78a6672ced
@@ -1,69 +0,0 @@
|
||||
# 053 — Move the log sort selector from Log Preview to the Timeline
|
||||
|
||||
## Summary
|
||||
|
||||
The "Sort by" control lives in the wrong component. It's currently
|
||||
rendered inside `LogPreview`'s own section header
|
||||
(`webapp/src/components/LogPreview.tsx:327-336`) as a `SortPicker`
|
||||
(`LogPreview.tsx:68-107`, a `<select>` plus an "Fall back to ottyl_seq"
|
||||
checkbox), with its `LogSortSettings` type and
|
||||
`loadLogSortSettings`/`saveLogSortSettings` persistence helpers
|
||||
(`LogPreview.tsx:37-66`, localStorage key `ottyl.logPreview.sort`) also
|
||||
defined there. `LogTimeline.tsx` has no sort-related prop at all
|
||||
(`LogTimeline.tsx:44-56`) — its header only has time-range preset buttons
|
||||
and a "Clear selection" link (`LogTimeline.tsx:118-149`).
|
||||
|
||||
The actual sort state is owned one level up, duplicated identically in
|
||||
`App.tsx`'s two top-level render modes: `FullApp`
|
||||
(`App.tsx:389-394`) and `StandaloneLog` (`App.tsx:263-268`), both via
|
||||
`useState<LogSortSettings>(loadLogSortSettings)` plus a
|
||||
`handleSortSettingsChange` callback. Both currently pass
|
||||
`sortSettings`/`onSortSettingsChange` only to `LogPreview`
|
||||
(`App.tsx:361`, `App.tsx:527`), never to `LogTimeline`
|
||||
(`App.tsx:237-243`, `354-360`/`520-526`). The setting already drives
|
||||
`fetchLogs` (`App.tsx:319-326`, `464-471`, dependency at `App.tsx:347`,
|
||||
`494`), which forwards `sortBy`/`sortMissingFallback` to
|
||||
`GET /api/logs` via `fetchLogs` in `webapp/src/lib/api.ts:242-262`. The
|
||||
backend already fully supports this — `src/dto.rs:86-90`'s
|
||||
`sort_by`/`sort_missing_fallback` fields, validated and applied in
|
||||
`src/engine.rs:1223-1281` (400 if `sort_by` isn't a declared param,
|
||||
`engine.rs:1227`; missing-value fallback behavior at `engine.rs:1260-1281`).
|
||||
The timeline query/DTO backing `fetchTimeline`
|
||||
(`webapp/src/lib/api.ts:264-274`) has no `sort_by` field and doesn't need
|
||||
one — this is a frontend-only relocation of where the control lives, not
|
||||
a backend change.
|
||||
|
||||
## Scope
|
||||
|
||||
- Move `SortPicker` (and ideally the `LogSortSettings` type plus
|
||||
`loadLogSortSettings`/`saveLogSortSettings`) out of `LogPreview.tsx`
|
||||
into `LogTimeline.tsx`, rendering it in the Timeline's header
|
||||
(`LogTimeline.tsx:118-149`) alongside the existing range controls.
|
||||
- Add `sortSettings`/`onSortSettingsChange` props to `LogTimeline`.
|
||||
- Update both `App.tsx` render sites (`FullApp` around line 237,
|
||||
`StandaloneLog` around line 354/520) to pass `sortSettings`/
|
||||
`onSortSettingsChange` to `LogTimeline` instead of `LogPreview`.
|
||||
- `LogPreview` keeps rendering the `logs` prop as already sorted by the
|
||||
parent's `fetchLogs` call — remove its own `SortPicker` usage and the
|
||||
now-unused sort-control markup from its header, but keep consuming
|
||||
`sortSettings` if it still needs read access to it (e.g. for a
|
||||
"sorted by X" label), or drop that too if not needed.
|
||||
- No backend changes — `sort_by`/`sort_missing_fallback` on
|
||||
`GET /api/logs` are unaffected.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- The "Sort by" selector and "fall back to ottyl_seq" checkbox appear in
|
||||
the Timeline component's header, not in Log Preview's.
|
||||
- Changing sort order from the Timeline's control changes the order of
|
||||
entries shown in Log Preview (same behavior as today, just relocated).
|
||||
- The chosen sort setting still persists across reloads (localStorage key
|
||||
can be renamed if it moves file, but must not silently reset to
|
||||
default on the first load after this change — migrate or fall back
|
||||
gracefully).
|
||||
- `make build` (webapp `tsc -b && vite build`) passes with no leftover
|
||||
unused imports/types in `LogPreview.tsx`.
|
||||
|
||||
## Depends on
|
||||
|
||||
None.
|
||||
+12
-3
@@ -6,10 +6,13 @@ import {
|
||||
TIME_RANGE_PRESETS,
|
||||
computeBucketMs,
|
||||
computePresetRange,
|
||||
loadLogSortSettings,
|
||||
saveLogSortSettings,
|
||||
type LogSortSettings,
|
||||
type TimeRange,
|
||||
type TimeRangePresetId,
|
||||
} from './components/LogTimeline'
|
||||
import { LogPreview, loadLogSortSettings, saveLogSortSettings, type LogSortSettings } from './components/LogPreview'
|
||||
import { LogPreview } from './components/LogPreview'
|
||||
import { Params } from './components/Params'
|
||||
import { Metrics } from './components/Metrics'
|
||||
import { Dashboards } from './components/Dashboards'
|
||||
@@ -357,8 +360,11 @@ function StandaloneLog() {
|
||||
onSelectRange={handleManualRangeSelect}
|
||||
activePreset={activePreset}
|
||||
onSelectPreset={selectPreset}
|
||||
paramNames={params.map((p) => p.name)}
|
||||
sortSettings={sortSettings}
|
||||
onSortSettingsChange={handleSortSettingsChange}
|
||||
/>
|
||||
<LogPreview logs={logs} params={params} sortSettings={sortSettings} onSortSettingsChange={handleSortSettingsChange} />
|
||||
<LogPreview logs={logs} params={params} />
|
||||
</div>
|
||||
</div>
|
||||
</StandalonePage>
|
||||
@@ -523,8 +529,11 @@ function FullApp() {
|
||||
onSelectRange={handleManualRangeSelect}
|
||||
activePreset={activePreset}
|
||||
onSelectPreset={selectPreset}
|
||||
paramNames={params.map((p) => p.name)}
|
||||
sortSettings={sortSettings}
|
||||
onSortSettingsChange={handleSortSettingsChange}
|
||||
/>
|
||||
<LogPreview logs={logs} params={params} sortSettings={sortSettings} onSortSettingsChange={handleSortSettingsChange} />
|
||||
<LogPreview logs={logs} params={params} />
|
||||
</div>
|
||||
{!logViewerCollapsed && <ResizeHandle onResize={handleLogViewerResize} onResizeEnd={handleLogViewerResizeEnd} />}
|
||||
<Params params={params} onDeclared={loadParams} />
|
||||
|
||||
@@ -25,87 +25,6 @@ function saveVisibleParamColumns(columns: string[]) {
|
||||
}
|
||||
}
|
||||
|
||||
// Ticket 045: ordering key + missing-param handling for the log preview / timeline. Scoped like
|
||||
// `VISIBLE_PARAM_COLUMNS_STORAGE_KEY` and `App.tsx`'s `LOG_VIEWER_HEIGHT_STORAGE_KEY` above — one
|
||||
// shared "log viewer" concept, so a single localStorage key covers both the inline viewer
|
||||
// (`FullApp`) and its popout (`StandaloneLog`) rather than each keeping its own copy, which would
|
||||
// surprise a user switching between the two. There's no dashboard/panel-scoped analog for this
|
||||
// setting to divide by (unlike e.g. per-dashboard panels), so a single global key matches how the
|
||||
// rest of the log viewer's own settings are scoped.
|
||||
const LOG_SORT_STORAGE_KEY = 'ottyl.logPreview.sort'
|
||||
|
||||
export interface LogSortSettings {
|
||||
/** An already-declared param name to order log lines by, or `null` for the default (`ottyl_ts`-tracking) order. */
|
||||
sortBy: string | null
|
||||
/** Only meaningful when `sortBy` is set. `true` (default) orders lines missing that param by `ottyl_seq`, appended after the sorted lines; `false` drops them from view. */
|
||||
sortMissingFallback: boolean
|
||||
}
|
||||
|
||||
const DEFAULT_LOG_SORT_SETTINGS: LogSortSettings = { sortBy: null, sortMissingFallback: true }
|
||||
|
||||
export function loadLogSortSettings(): LogSortSettings {
|
||||
try {
|
||||
const raw = localStorage.getItem(LOG_SORT_STORAGE_KEY)
|
||||
if (!raw) return DEFAULT_LOG_SORT_SETTINGS
|
||||
const parsed = JSON.parse(raw)
|
||||
if (!parsed || typeof parsed !== 'object') return DEFAULT_LOG_SORT_SETTINGS
|
||||
const sortBy = typeof parsed.sortBy === 'string' ? parsed.sortBy : null
|
||||
const sortMissingFallback = typeof parsed.sortMissingFallback === 'boolean' ? parsed.sortMissingFallback : true
|
||||
return { sortBy, sortMissingFallback }
|
||||
} catch {
|
||||
return DEFAULT_LOG_SORT_SETTINGS
|
||||
}
|
||||
}
|
||||
|
||||
export function saveLogSortSettings(settings: LogSortSettings) {
|
||||
try {
|
||||
localStorage.setItem(LOG_SORT_STORAGE_KEY, JSON.stringify(settings))
|
||||
} catch {
|
||||
// best-effort — private browsing or full storage just means the selection won't persist across reloads
|
||||
}
|
||||
}
|
||||
|
||||
function SortPicker({
|
||||
paramNames,
|
||||
settings,
|
||||
onChange,
|
||||
}: {
|
||||
paramNames: string[]
|
||||
settings: LogSortSettings
|
||||
onChange: (next: LogSortSettings) => void
|
||||
}) {
|
||||
return (
|
||||
<div className="flex items-center gap-2 text-xs text-text-secondary">
|
||||
<label className="flex items-center gap-1">
|
||||
Sort by
|
||||
<select
|
||||
value={settings.sortBy ?? ''}
|
||||
onChange={(e) => onChange({ ...settings, sortBy: e.target.value || null })}
|
||||
className="rounded border border-border-soft bg-surface-0 px-1 py-0.5 text-text"
|
||||
>
|
||||
<option value="">ottyl_ts (default)</option>
|
||||
{paramNames.map((name) => (
|
||||
<option key={name} value={name}>
|
||||
{name}
|
||||
</option>
|
||||
))}
|
||||
</select>
|
||||
</label>
|
||||
{settings.sortBy && (
|
||||
<label className="flex cursor-pointer items-center gap-1" title="How lines missing the sort param are handled">
|
||||
<input
|
||||
type="checkbox"
|
||||
checked={settings.sortMissingFallback}
|
||||
onChange={(e) => onChange({ ...settings, sortMissingFallback: e.target.checked })}
|
||||
className="accent-text-muted"
|
||||
/>
|
||||
Fall back to ottyl_seq
|
||||
</label>
|
||||
)}
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
||||
function ColumnPicker({
|
||||
paramNames,
|
||||
visible,
|
||||
@@ -293,14 +212,10 @@ function useScrollAnchor(containerRef: React.RefObject<HTMLDivElement | null>, d
|
||||
export function LogPreview({
|
||||
logs,
|
||||
params,
|
||||
sortSettings,
|
||||
onSortSettingsChange,
|
||||
}: {
|
||||
logs: LogEntry[]
|
||||
/** Ticket 045/053: already sorted by the caller's `fetchLogs` call per its own sort setting (owned and rendered up in `LogTimeline`'s header now) — this component just renders whatever order it's given. */
|
||||
params: ParamDeclaration[]
|
||||
/** Ticket 045: current ordering choice, owned by the caller (it also feeds `fetchLogs`) rather than internally like `visibleParamColumns` — see `App.tsx`'s poll loop. */
|
||||
sortSettings: LogSortSettings
|
||||
onSortSettingsChange: (next: LogSortSettings) => void
|
||||
}) {
|
||||
const paramNames = params.map((d) => d.name)
|
||||
const [visibleParamColumns, setVisibleParamColumns] = useState<string[]>(loadVisibleParamColumns)
|
||||
@@ -330,7 +245,6 @@ export function LogPreview({
|
||||
<HelpIcon sectionId="log-preview" label="Help: Log preview" />
|
||||
</div>
|
||||
<div className="flex items-center gap-3">
|
||||
<SortPicker paramNames={paramNames} settings={sortSettings} onChange={onSortSettingsChange} />
|
||||
<ColumnPicker paramNames={paramNames} visible={visible} onToggle={toggleColumn} />
|
||||
</div>
|
||||
</div>
|
||||
|
||||
@@ -41,18 +41,116 @@ function formatTimestamp(ms: number): string {
|
||||
return new Date(ms).toLocaleTimeString()
|
||||
}
|
||||
|
||||
// Ticket 053: moved here from LogPreview.tsx, along with the `SortPicker` control below — the
|
||||
// setting now renders in the Timeline's header, but it still drives `fetchLogs` up in App.tsx the
|
||||
// same way it always did. Renamed from the old `ottyl.logPreview.sort` key since the control no
|
||||
// longer lives in "log preview"; `loadLogSortSettings` still falls back to the old key so an
|
||||
// existing choice survives this move instead of silently resetting on first load.
|
||||
const LOG_SORT_STORAGE_KEY = 'ottyl.logTimeline.sort'
|
||||
const LEGACY_LOG_SORT_STORAGE_KEY = 'ottyl.logPreview.sort'
|
||||
|
||||
export interface LogSortSettings {
|
||||
/** An already-declared param name to order log lines by, or `null` for the default (`ottyl_ts`-tracking) order. */
|
||||
sortBy: string | null
|
||||
/** Only meaningful when `sortBy` is set. `true` (default) orders lines missing that param by `ottyl_seq`, appended after the sorted lines; `false` drops them from view. */
|
||||
sortMissingFallback: boolean
|
||||
}
|
||||
|
||||
const DEFAULT_LOG_SORT_SETTINGS: LogSortSettings = { sortBy: null, sortMissingFallback: true }
|
||||
|
||||
function parseLogSortSettings(raw: string | null): LogSortSettings | null {
|
||||
if (!raw) return null
|
||||
try {
|
||||
const parsed = JSON.parse(raw)
|
||||
if (!parsed || typeof parsed !== 'object') return null
|
||||
const sortBy = typeof parsed.sortBy === 'string' ? parsed.sortBy : null
|
||||
const sortMissingFallback = typeof parsed.sortMissingFallback === 'boolean' ? parsed.sortMissingFallback : true
|
||||
return { sortBy, sortMissingFallback }
|
||||
} catch {
|
||||
return null
|
||||
}
|
||||
}
|
||||
|
||||
export function loadLogSortSettings(): LogSortSettings {
|
||||
try {
|
||||
return (
|
||||
parseLogSortSettings(localStorage.getItem(LOG_SORT_STORAGE_KEY)) ??
|
||||
parseLogSortSettings(localStorage.getItem(LEGACY_LOG_SORT_STORAGE_KEY)) ??
|
||||
DEFAULT_LOG_SORT_SETTINGS
|
||||
)
|
||||
} catch {
|
||||
return DEFAULT_LOG_SORT_SETTINGS
|
||||
}
|
||||
}
|
||||
|
||||
export function saveLogSortSettings(settings: LogSortSettings) {
|
||||
try {
|
||||
localStorage.setItem(LOG_SORT_STORAGE_KEY, JSON.stringify(settings))
|
||||
} catch {
|
||||
// best-effort — private browsing or full storage just means the selection won't persist across reloads
|
||||
}
|
||||
}
|
||||
|
||||
function SortPicker({
|
||||
paramNames,
|
||||
settings,
|
||||
onChange,
|
||||
}: {
|
||||
paramNames: string[]
|
||||
settings: LogSortSettings
|
||||
onChange: (next: LogSortSettings) => void
|
||||
}) {
|
||||
return (
|
||||
<div className="flex items-center gap-2 text-xs text-text-secondary">
|
||||
<label className="flex items-center gap-1">
|
||||
Sort by
|
||||
<select
|
||||
value={settings.sortBy ?? ''}
|
||||
onChange={(e) => onChange({ ...settings, sortBy: e.target.value || null })}
|
||||
className="rounded border border-border-soft bg-surface-0 px-1 py-0.5 text-text"
|
||||
>
|
||||
<option value="">ottyl_ts (default)</option>
|
||||
{paramNames.map((name) => (
|
||||
<option key={name} value={name}>
|
||||
{name}
|
||||
</option>
|
||||
))}
|
||||
</select>
|
||||
</label>
|
||||
{settings.sortBy && (
|
||||
<label className="flex cursor-pointer items-center gap-1" title="How lines missing the sort param are handled">
|
||||
<input
|
||||
type="checkbox"
|
||||
checked={settings.sortMissingFallback}
|
||||
onChange={(e) => onChange({ ...settings, sortMissingFallback: e.target.checked })}
|
||||
className="accent-text-muted"
|
||||
/>
|
||||
Fall back to ottyl_seq
|
||||
</label>
|
||||
)}
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
||||
export function LogTimeline({
|
||||
buckets,
|
||||
range,
|
||||
onSelectRange,
|
||||
activePreset,
|
||||
onSelectPreset,
|
||||
paramNames,
|
||||
sortSettings,
|
||||
onSortSettingsChange,
|
||||
}: {
|
||||
buckets: TimelineBucket[]
|
||||
range: TimeRange | null
|
||||
onSelectRange: (range: TimeRange | null) => void
|
||||
activePreset: TimeRangePresetId | null
|
||||
onSelectPreset: (id: TimeRangePresetId) => void
|
||||
/** Ticket 045/053: current ordering choice, owned by the caller (it also feeds `fetchLogs`). Omitted by callers with no log list to sort (e.g. the standalone metrics/dashboards popout). */
|
||||
paramNames?: string[]
|
||||
sortSettings?: LogSortSettings
|
||||
onSortSettingsChange?: (next: LogSortSettings) => void
|
||||
}) {
|
||||
const containerRef = useRef<HTMLDivElement>(null)
|
||||
const [dragStartX, setDragStartX] = useState<number | null>(null)
|
||||
@@ -122,7 +220,11 @@ export function LogTimeline({
|
||||
Timeline
|
||||
<HelpIcon sectionId="timeline" label="Help: Timeline" />
|
||||
</span>
|
||||
<div className="flex items-center gap-1.5 text-xs text-text-subtle">
|
||||
<div className="flex items-center gap-3 text-xs text-text-subtle">
|
||||
{sortSettings && onSortSettingsChange && (
|
||||
<SortPicker paramNames={paramNames ?? []} settings={sortSettings} onChange={onSortSettingsChange} />
|
||||
)}
|
||||
<div className="flex items-center gap-1.5">
|
||||
<div className="flex items-center gap-1">
|
||||
{TIME_RANGE_PRESETS.map((preset) => (
|
||||
<button
|
||||
@@ -147,6 +249,7 @@ export function LogTimeline({
|
||||
)}
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
<div className="p-3">
|
||||
<div
|
||||
ref={containerRef}
|
||||
|
||||
Reference in New Issue
Block a user