From aba1f7384d44d020ce8e4d9036c7cececb3c6c13 Mon Sep 17 00:00:00 2001 From: johnnyjoygh Date: Wed, 2 Sep 2026 22:50:29 +0800 Subject: [PATCH] feat(sidebar): separate checked filters from current rows The rail used one fill for two meanings: the page you are on and a filter that is on. A view, a tag and a calendar day could light three rows like three current pages, while the view itself was never echoed above the list. - Split sidebarRowStateClasses into idle / current / checked. Nav pills, settings sections and list scopes stay filled; views, tags (flat and tree) and the selected calendar day take the checked look: foreground text, accent icon and count, a 2px accent mark in the rail inset, no surface. - Echo the active view as a filter chip like a tag, and format the day chip as a date. - Make the tags list/tree toggle surface-free so only rows carry a fill. - Tighten rows from 32px to 28px; nav pills keep their 32px square. --- web/src/components/ActivityCalendar/utils.ts | 3 +- web/src/components/AppSidebar/AppSidebar.tsx | 17 ++- .../AppSidebar/CommonSidebarContent.tsx | 2 +- web/src/components/AppSidebar/SidebarRow.tsx | 41 +++++-- .../components/AppSidebar/SidebarSection.tsx | 3 +- web/src/components/AppSidebar/TagsSection.tsx | 12 +- .../components/AppSidebar/ViewsSection.tsx | 12 +- .../components/AppSidebar/sidebar-layout.ts | 2 +- web/src/components/MemoFilters.tsx | 81 +++++++++---- .../PagedMemoList/PagedMemoList.tsx | 4 +- web/src/components/TagTree.tsx | 7 +- .../calendar-cell-empty-clickable.test.tsx | 14 ++- web/tests/sidebar-row-grammar.test.tsx | 2 +- web/tests/sidebar-row-state.test.tsx | 108 ++++++++++++++++++ 14 files changed, 251 insertions(+), 57 deletions(-) create mode 100644 web/tests/sidebar-row-state.test.tsx diff --git a/web/src/components/ActivityCalendar/utils.ts b/web/src/components/ActivityCalendar/utils.ts index 51d9b8fa..4bb8e698 100644 --- a/web/src/components/ActivityCalendar/utils.ts +++ b/web/src/components/ActivityCalendar/utils.ts @@ -24,7 +24,8 @@ export const getCellIntensityClass = (day: CalendarDayCell, maxCount: number): s }; export const getCalendarCellStateClass = (day: Pick): string => - day.isSelected ? "z-10 ring-2 ring-blue-500/70 ring-inset" : ""; + // A picked day is a checked filter like a view or tag row: it takes the accent, not a ring. + day.isSelected ? "z-10 bg-primary font-medium text-primary-foreground" : ""; export const generateMonthsForYear = (year: number): string[] => { return Array.from({ length: MONTHS_IN_YEAR }, (_, i) => dayjs(`${year}-01-01`).add(i, "month").format("YYYY-MM")); diff --git a/web/src/components/AppSidebar/AppSidebar.tsx b/web/src/components/AppSidebar/AppSidebar.tsx index c9439ea5..39eca063 100644 --- a/web/src/components/AppSidebar/AppSidebar.tsx +++ b/web/src/components/AppSidebar/AppSidebar.tsx @@ -108,8 +108,13 @@ const ProfileMode = () => { return ( - setMode("memos")} /> - setMode("map")} /> + setMode("memos")} + /> + setMode("map")} /> ); }; @@ -191,7 +196,7 @@ const AttachmentsSidebarContent = () => { {rows.map((row) => ( { {rows.map((row) => ( { key={section.key} to={`${ROUTES.SETTING}#${section.key}`} onClick={() => setMobileOpen(false)} - className={cn(SIDEBAR_ROW_CLASSES, sidebarRowStateClasses(currentSection === section.key))} + className={cn(SIDEBAR_ROW_CLASSES, sidebarRowStateClasses(currentSection === section.key ? "current" : "idle"))} > {t(section.labelKey)} @@ -324,7 +329,7 @@ interface GlobalNavItem { * label only opens the text track, so the artwork and surface never jump. */ const navPillClasses = (active: boolean) => - cn(sidebarSurfaceVariants({ role: "navPill" }), SIDEBAR_ROW_FOCUS_CLASSES, sidebarRowStateClasses(active)); + cn(sidebarSurfaceVariants({ role: "navPill" }), SIDEBAR_ROW_FOCUS_CLASSES, sidebarRowStateClasses(active ? "current" : "idle")); const NavPillLabel = ({ expanded, label, children }: { expanded: boolean; label: ReactNode; children?: ReactNode }) => ( { to={ROUTES.ABOUT} aria-current={aboutActive ? "page" : undefined} onClick={() => setMobileOpen(false)} - className={cn(SIDEBAR_ROW_CLASSES, sidebarRowStateClasses(aboutActive))} + className={cn(SIDEBAR_ROW_CLASSES, sidebarRowStateClasses(aboutActive ? "current" : "idle"))} > {t("common.about")} diff --git a/web/src/components/AppSidebar/SidebarRow.tsx b/web/src/components/AppSidebar/SidebarRow.tsx index 2024a515..a931579f 100644 --- a/web/src/components/AppSidebar/SidebarRow.tsx +++ b/web/src/components/AppSidebar/SidebarRow.tsx @@ -18,8 +18,8 @@ export const SIDEBAR_ROW_FOCUS_CLASSES = export const SIDEBAR_ROW_CLASSES = `${SIDEBAR_ROW_BOX_CLASSES} ${SIDEBAR_ROW_FOCUS_CLASSES}`; -export const SIDEBAR_ROW_ICON_CLASSES = "me-auto size-4 shrink-0 opacity-75"; -const SIDEBAR_ROW_COUNT_CLASSES = "text-2xs tabular-nums text-muted-foreground/60"; +export const SIDEBAR_ROW_ICON_CLASSES = "me-auto size-4 shrink-0 opacity-75 group-data-checked:text-primary group-data-checked:opacity-100"; +const SIDEBAR_ROW_COUNT_CLASSES = "text-2xs tabular-nums text-muted-foreground/60 group-data-checked:text-primary"; /** * The focusable body of a split row — rows whose box is a wrapper carrying other controls @@ -48,14 +48,32 @@ export const SidebarRowIconSlot = ({ icon: Icon }: { icon: LucideIcon }) => ( ); -/** Idle and selected colouring for a row box, kept in one place so lists cannot drift apart. */ -export const sidebarRowStateClasses = (active?: boolean) => - active - ? "bg-sidebar-accent font-medium text-sidebar-accent-foreground" - : "text-muted-foreground hover:bg-sidebar-accent/65 hover:text-foreground"; +/** + * One selected look per meaning, kept together so lists cannot drift apart. + * `current` is the place you are (nav pills, settings sections, list scopes) and fills the + * row. `checked` is a filter that is on (a view, a tag) and must never read as a page: no + * surface, foreground text, an accent mark on the rail edge, accent icon and count. + */ +export type SidebarRowState = "idle" | "current" | "checked"; + +const SIDEBAR_ROW_HOVER_CLASSES = "hover:bg-sidebar-accent/65 hover:text-foreground"; + +export const sidebarRowStateClasses = (state: SidebarRowState = "idle") => { + if (state === "current") return "bg-sidebar-accent font-medium text-sidebar-accent-foreground"; + if (state === "checked") { + // The mark hangs in the rail's 12px inset so the label stays on its rail. + return `relative font-medium text-foreground ${SIDEBAR_ROW_HOVER_CLASSES} before:absolute before:inset-y-1.5 before:-start-3 before:w-0.5 before:rounded-e-full before:bg-primary before:content-['']`; + } + return `text-muted-foreground ${SIDEBAR_ROW_HOVER_CLASSES}`; +}; + +/** Goes on the row box so the icon slot and count rail inside it can take the checked colour. */ +export const sidebarRowStateAttributes = (state: SidebarRowState) => ({ + "data-checked": state === "checked" ? "" : undefined, +}); interface Props { - active?: boolean; + state?: SidebarRowState; icon?: LucideIcon; label: ReactNode; count?: number; @@ -63,12 +81,13 @@ interface Props { trailing?: ReactNode; } -const SidebarRow = ({ active, icon: Icon, label, count, onClick, trailing }: Props) => ( +const SidebarRow = ({ state = "idle", icon: Icon, label, count, onClick, trailing }: Props) => ( + + +); + const MemoFilters = ({ className }: { className?: string }) => { const t = useTranslate(); - const { filters, removeFilter } = useMemoFilterContext(); + const location = useLocation(); + const currentUser = useCurrentUser(); + const { filters, memoView, removeFilter, setMemoView } = useMemoFilterContext(); + // A remembered view only narrows the collection routes; elsewhere it is dormant and must not be echoed. + const viewApplies = memoView !== undefined && isMemoScopeRoute(location.pathname); + const { data: memoViews = [] } = useMemoViews(viewApplies ? currentUser?.name : undefined); const handleRemoveFilter = (filter: MemoFilter) => { removeFilter((f: MemoFilter) => isEqual(f, filter)); @@ -77,31 +113,28 @@ const MemoFilters = ({ className }: { className?: string }) => { return config.getLabel(filter.value, t); }; - if (filters.length === 0) { + const viewChip = (() => { + if (!viewApplies) return null; + if (memoView === BUILTIN_TASKS_VIEW_ID) return { icon: SquareCheckIcon, label: t("common.tasks") }; + const title = memoViews.find((item) => getMemoViewId(item.name) === memoView)?.title; + return title ? { icon: ParenthesesIcon, label: title } : null; + })(); + + if (filters.length === 0 && !viewChip) { return null; } return (
- {filters.map((filter) => { - const config = FILTER_CONFIGS[filter.factor]; - const Icon = config?.icon; - - return ( -
- {Icon && } - {getFilterDisplayText(filter)} - - - -
- ); - })} + {viewChip && setMemoView(undefined)} />} + {filters.map((filter) => ( + handleRemoveFilter(filter)} + /> + ))}
); }; diff --git a/web/src/components/PagedMemoList/PagedMemoList.tsx b/web/src/components/PagedMemoList/PagedMemoList.tsx index 3c04b443..05541bb2 100644 --- a/web/src/components/PagedMemoList/PagedMemoList.tsx +++ b/web/src/components/PagedMemoList/PagedMemoList.tsx @@ -116,7 +116,7 @@ function useAutoFetchWhenNotScrollable({ const PagedMemoList = (props: Props) => { const t = useTranslate(); const { isUserSettingsInitialized } = useAuth(); - const { filters } = useMemoFilterContext(); + const { filters, memoView } = useMemoFilterContext(); const { maxColumns, compactMode } = useView(); // maxColumns is a ceiling: 1 = single reading column, 0 = as many as fit. The single // column renders in normal document flow; anything wider becomes the packed grid. @@ -227,7 +227,7 @@ const PagedMemoList = (props: Props) => { // empty state follows them. The newest memo also lands directly beneath them (priorityKey // above). Every vertical seam inside the stack uses GRID_GAP so y-spacing matches the // grid's x-spacing exactly. - const hasFilters = filters.length > 0; + const hasFilters = filters.length > 0 || memoView !== undefined; const gridLeading = leadingContent || hasFilters || initialLoader || emptyPlaceholder ? (
diff --git a/web/src/components/TagTree.tsx b/web/src/components/TagTree.tsx index 62da6fd4..e5b4fc4f 100644 --- a/web/src/components/TagTree.tsx +++ b/web/src/components/TagTree.tsx @@ -8,6 +8,7 @@ import { SIDEBAR_ROW_SLOT_BUTTON_CLASSES, SIDEBAR_ROW_SLOT_CLASSES, SidebarRowIconSlot, + sidebarRowStateAttributes, sidebarRowStateClasses, } from "@/components/AppSidebar/SidebarRow"; import { useLocalStorage, useOverflowTitle } from "@/hooks"; @@ -41,7 +42,7 @@ interface TagTreeExpansion { const EMPTY_EXPANSION: TagTreeExpansion = { expanded: [] }; // A structural row toggles like any other, so it hovers like one too — just quieter at rest. -const STRUCTURAL_ROW_CLASSES = cn(sidebarRowStateClasses(false), "font-medium text-muted-foreground/65"); +const STRUCTURAL_ROW_CLASSES = cn(sidebarRowStateClasses(), "font-medium text-muted-foreground/65"); /** One announcement for a tag row in either layout, so tree and flat mode never drift apart. */ export const tagRowAriaLabel = (t: ReturnType, tag: string, amount: number) => @@ -121,6 +122,7 @@ const TagItem = ({ tag, depth, activeTag, expanded, onTagClick, onToggle }: TagI const open = hasSubTags && expanded.has(tag.text); const { ref: labelRef, title } = useOverflowTitle(isTag ? `#${tag.text}` : tag.text); const tagLabel = tag.amount !== undefined ? tagRowAriaLabel(t, tag.text, tag.amount) : undefined; + const state = isActive ? "checked" : "idle"; return (
@@ -129,9 +131,10 @@ const TagItem = ({ tag, depth, activeTag, expanded, onTagClick, onToggle }: TagI aria-level={depth + 1} aria-selected={isActive || undefined} aria-expanded={hasSubTags ? open : undefined} + {...sidebarRowStateAttributes(state)} className={cn( SIDEBAR_ROW_BOX_CLASSES, - isTag ? sidebarRowStateClasses(isActive) : STRUCTURAL_ROW_CLASSES, + isTag ? sidebarRowStateClasses(state) : STRUCTURAL_ROW_CLASSES, isAncestorOfActiveTag && !isActive && "text-foreground/75", )} // Overrides the start half of the box's `px-2`, leaving the trailing 8px intact. diff --git a/web/tests/calendar-cell-empty-clickable.test.tsx b/web/tests/calendar-cell-empty-clickable.test.tsx index 7adf0ef2..86fc3186 100644 --- a/web/tests/calendar-cell-empty-clickable.test.tsx +++ b/web/tests/calendar-cell-empty-clickable.test.tsx @@ -55,14 +55,24 @@ describe("CalendarCell empty-day clickability", () => { expect(button.querySelector('[aria-hidden="true"]')).toHaveClass("rounded-full"); }); - it("uses an inset ring for selection without changing the numeral weight", () => { + it("fills a selected day with the accent like a checked filter row, keeping the numeral weight", () => { render( {}} />); const button = screen.getByRole("button", { name: /selected/ }); - expect(chipOf(button)).toHaveClass("ring-2", "ring-inset"); + expect(chipOf(button)).toHaveClass("bg-primary", "text-primary-foreground", "font-medium"); + expect(chipOf(button)).not.toHaveClass("ring-2", "ring-inset"); expect(chipOf(button)).not.toHaveClass("font-semibold", "font-bold"); }); + it("keeps the accent fill on a selected empty day under hover", () => { + render( {}} />); + + const chip = chipOf(screen.getByRole("button", { name: /selected/ })); + // The empty-cell hover tint would replace bg-primary on hover and strand the light numeral. + expect(chip).not.toHaveClass("group-hover/day:bg-muted/40", "bg-transparent"); + expect(chip).toHaveClass("bg-primary"); + }); + it("caps the chip so a wider container buys hit area, not calendar height", () => { render( {}} />); diff --git a/web/tests/sidebar-row-grammar.test.tsx b/web/tests/sidebar-row-grammar.test.tsx index 63b24fb5..35b79f00 100644 --- a/web/tests/sidebar-row-grammar.test.tsx +++ b/web/tests/sidebar-row-grammar.test.tsx @@ -39,7 +39,7 @@ describe("sidebar row grammar", () => { const row = screen.getByRole("button", { name: "Tasks3" }); expect(row).toHaveClass(...boxClasses); - expect(row).toHaveClass("h-8", "w-full", "gap-1", "rounded-md", "px-2"); + expect(row).toHaveClass("h-7", "w-full", "gap-1", "rounded-md", "px-2"); expect(row).not.toHaveClass("-mx-1"); // Icon in the shared slot and count in the shared rail, so every list — nav rows, // views, tags in both modes — keeps its icons and digits on the same vertical lines. diff --git a/web/tests/sidebar-row-state.test.tsx b/web/tests/sidebar-row-state.test.tsx new file mode 100644 index 00000000..53605786 --- /dev/null +++ b/web/tests/sidebar-row-state.test.tsx @@ -0,0 +1,108 @@ +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { fireEvent, render, screen } from "@testing-library/react"; +import { HashIcon } from "lucide-react"; +import { useEffect } from "react"; +import { MemoryRouter } from "react-router-dom"; +import { describe, expect, it, vi } from "vitest"; +import SidebarRow, { sidebarRowStateClasses } from "@/components/AppSidebar/SidebarRow"; +import { SIDEBAR_SECTION_ACTION_ACTIVE_CLASSES } from "@/components/AppSidebar/SidebarSection"; +import TagsSection from "@/components/AppSidebar/TagsSection"; +import MemoFilters from "@/components/MemoFilters"; +import { MemoFilterProvider, useMemoFilterContext } from "@/contexts/MemoFilterContext"; +import { BUILTIN_TASKS_VIEW_ID } from "@/lib/memo-views"; + +vi.mock("@/utils/i18n", () => ({ useTranslate: () => (key: string) => key })); +vi.mock("@/hooks/useCurrentUser", () => ({ default: () => ({ name: "users/1" }) })); +vi.mock("@/hooks/useUserQueries", () => ({ + useMemoViews: () => ({ data: [{ name: "users/1/memoViews/abc", title: "Last week", filter: "pinned" }] }), +})); + +const tokens = (classes: string) => classes.split(" "); + +/** + * The rail has two selected meanings and each gets its own look: a filled row is the page + * you are on, a checked row is a filter that is on. Views, tags and days are filters, so + * they must never wear the fill, and every filter that is on must be echoed above the list. + */ +describe("sidebar selected grammar", () => { + it("fills only a current row", () => { + expect(tokens(sidebarRowStateClasses("current"))).toContain("bg-sidebar-accent"); + expect(tokens(sidebarRowStateClasses("checked"))).not.toContain("bg-sidebar-accent"); + expect(tokens(sidebarRowStateClasses("checked"))).toContain("before:bg-primary"); + expect(tokens(sidebarRowStateClasses())).not.toContain("font-medium"); + }); + + it("marks a checked row so its icon and count take the accent", () => { + render(); + + const row = screen.getByRole("button", { name: "work4" }); + expect(row).toHaveAttribute("data-checked"); + expect(row).toHaveAttribute("aria-pressed", "true"); + expect(row).not.toHaveClass("bg-sidebar-accent"); + expect(row.querySelector("svg")).toHaveClass("group-data-checked:text-primary"); + expect(screen.getByText("4")).toHaveClass("group-data-checked:text-primary"); + }); + + it("checks a tag row instead of filling it", () => { + render( + + + + + , + ); + + const row = screen.getByRole("button", { name: "#work, setting.tags.used-count" }); + expect(row).not.toHaveAttribute("data-checked"); + fireEvent.click(row); + expect(row).toHaveAttribute("data-checked"); + expect(row).not.toHaveClass("bg-sidebar-accent"); + }); + + it("keeps section mode toggles surface-free", () => { + expect(tokens(SIDEBAR_SECTION_ACTION_ACTIVE_CLASSES)).not.toContain("bg-sidebar-accent"); + }); +}); + +const SelectView = ({ id }: { id: string }) => { + const { setMemoView } = useMemoFilterContext(); + useEffect(() => setMemoView(id), [id, setMemoView]); + return null; +}; + +const renderChips = (path: string, viewId?: string) => + render( + + + + {viewId && } + + + + , + ); + +describe("MemoFilters", () => { + it("echoes the active view like any other filter, and clears it from the chip", () => { + renderChips("/", BUILTIN_TASKS_VIEW_ID); + + expect(screen.getByText("common.tasks")).toBeInTheDocument(); + fireEvent.click(screen.getByRole("button", { name: "Remove filter" })); + expect(screen.queryByText("common.tasks")).not.toBeInTheDocument(); + }); + + it("names a saved view by its title", () => { + renderChips("/", "abc"); + expect(screen.getByText("Last week")).toBeInTheDocument(); + }); + + it("stays quiet about a view off the collection routes, where it does not apply", () => { + renderChips("/u/alice", BUILTIN_TASKS_VIEW_ID); + expect(screen.queryByText("common.tasks")).not.toBeInTheDocument(); + }); + + it("formats a day filter as a date rather than the raw value", () => { + renderChips("/?filter=displayTime:2026-09-02"); + expect(screen.getByText("Sep 2, 2026")).toBeInTheDocument(); + }); +});