fix: more audit findings — perf, types, and a11y polish
- backend/photos: collapse the per-tag subquery loop in the tag filter
into a single GROUP BY ... HAVING COUNT(DISTINCT) = N subquery so the
cost is independent of how many tags the user is filtering on.
- useFilterUrlSync: type the parseUrl return value as
Partial<FilterState> & { currentSection?: string } so the section field
doesn't need an (out as any) cast.
- Timeline sticky header: bump opacity, padding, and border so it reads
more clearly against the underlying grid.
- FilterPill clear: convert the nested <button> (invalid HTML — buttons
cannot nest) to a span with role=button + keyboard handler, with a
larger hit area.
- RightSidebar: add aria-label to the close-X buttons so screen readers
announce them.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -146,17 +146,21 @@ async def list_photos(
|
||||
)
|
||||
|
||||
# Tag filter — comma-separated tag ids, AND semantics. A photo must
|
||||
# have a row in photo_tags for EVERY listed tag. Implemented as one
|
||||
# subquery per tag id since SQLite doesn't have an efficient
|
||||
# "set-contains-all" operator.
|
||||
# have a row in photo_tags for EVERY listed tag. Implemented as a
|
||||
# single GROUP BY ... HAVING COUNT(DISTINCT) = N subquery so the cost
|
||||
# is independent of the number of tags being filtered.
|
||||
if tag_ids:
|
||||
tag_id_list = [t.strip() for t in tag_ids.split(',') if t.strip()]
|
||||
for tid in tag_id_list:
|
||||
filters.append(
|
||||
Photo.id.in_(
|
||||
select(photo_tags.c.photo_id).where(photo_tags.c.tag_id == tid)
|
||||
if tag_id_list:
|
||||
matching_photos = (
|
||||
select(photo_tags.c.photo_id)
|
||||
.where(photo_tags.c.tag_id.in_(tag_id_list))
|
||||
.group_by(photo_tags.c.photo_id)
|
||||
.having(
|
||||
func.count(func.distinct(photo_tags.c.tag_id)) == len(tag_id_list)
|
||||
)
|
||||
)
|
||||
filters.append(Photo.id.in_(matching_photos))
|
||||
|
||||
# Apply all filters
|
||||
if filters:
|
||||
|
||||
@@ -106,17 +106,26 @@ export function FilterPill({
|
||||
<span className="font-mono text-[11px] opacity-90">{value}</span>
|
||||
)}
|
||||
{isActive && onClear ? (
|
||||
<button
|
||||
<span
|
||||
role="button"
|
||||
tabIndex={0}
|
||||
onClick={(e) => {
|
||||
e.stopPropagation()
|
||||
onClear()
|
||||
}}
|
||||
className="ml-0.5 rounded-full p-0.5 hover:bg-primary/30"
|
||||
onKeyDown={(e) => {
|
||||
if (e.key === 'Enter' || e.key === ' ') {
|
||||
e.preventDefault()
|
||||
e.stopPropagation()
|
||||
onClear()
|
||||
}
|
||||
}}
|
||||
className="ml-1 inline-flex h-5 w-5 cursor-pointer items-center justify-center rounded-full hover:bg-primary/30"
|
||||
title={`Clear ${label}`}
|
||||
aria-label={`Clear ${label}`}
|
||||
>
|
||||
<X className="h-3 w-3" />
|
||||
</button>
|
||||
</span>
|
||||
) : (
|
||||
<ChevronDown className="h-3 w-3 opacity-60" />
|
||||
)}
|
||||
|
||||
@@ -97,6 +97,7 @@ export function RightSidebar() {
|
||||
onClick={clearSelection}
|
||||
className="rounded p-1 text-text-muted hover:bg-surface-2 hover:text-text"
|
||||
title="Clear selection"
|
||||
aria-label="Clear selection"
|
||||
>
|
||||
<X className="h-4 w-4" />
|
||||
</button>
|
||||
|
||||
@@ -422,7 +422,7 @@ export function Timeline() {
|
||||
* positioned children so it isn't affected by translateY transforms.
|
||||
* Updates as the user scrolls past month boundaries. */}
|
||||
{stickyLabel && (
|
||||
<div className="pointer-events-none absolute left-0 right-0 top-0 z-20 border-b border-border bg-bg/90 px-4 py-1 backdrop-blur-sm">
|
||||
<div className="pointer-events-none absolute left-0 right-0 top-0 z-20 border-b-2 border-border bg-bg/95 px-4 py-1.5 shadow-sm backdrop-blur">
|
||||
<h3 className="text-sm font-semibold uppercase tracking-wide text-text">
|
||||
{stickyLabel}
|
||||
</h3>
|
||||
|
||||
@@ -28,9 +28,14 @@ const ALLOWED_SORT_FIELDS: SortField[] = [
|
||||
]
|
||||
const ALLOWED_SORT_ORDERS: SortOrder[] = ['asc', 'desc']
|
||||
|
||||
function parseUrl(): Partial<FilterState> {
|
||||
// What parseUrl returns: a partial filter state, plus the optional
|
||||
// section id (which lives on the store but isn't part of FilterState
|
||||
// itself). The hydrate action accepts this exact shape.
|
||||
type HydratePayload = Partial<FilterState> & { currentSection?: string }
|
||||
|
||||
function parseUrl(): HydratePayload {
|
||||
const sp = new URLSearchParams(window.location.search)
|
||||
const out: Partial<FilterState> = {}
|
||||
const out: HydratePayload = {}
|
||||
|
||||
const q = sp.get('q')
|
||||
if (q) out.q = q
|
||||
@@ -83,7 +88,7 @@ function parseUrl(): Partial<FilterState> {
|
||||
if (groupBy === 'date' || groupBy === 'tag') out.groupBy = groupBy
|
||||
|
||||
const section = sp.get('section')
|
||||
if (section) (out as any).currentSection = section
|
||||
if (section) out.currentSection = section
|
||||
|
||||
const sortBy = sp.get('sort')
|
||||
if (sortBy && ALLOWED_SORT_FIELDS.includes(sortBy as SortField)) {
|
||||
|
||||
Reference in New Issue
Block a user