diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 353a62c..c7cf380 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -35,6 +35,7 @@ pnpm format # Prettier --write - **Keep `domain/` pure and deterministic.** No `new Date()`/`Math.random()` in logic — take `now`/`today` as parameters and let the injected clock flow in. Zod schemas in `domain/model.ts` are the single source of truth; derive types from them. - **Tests live next to code** (`*.test.ts[x]`). Add or update tests with every change; coverage floors are strictest in `domain/`. Prefer driving behavior through public APIs over asserting internals. - **Accessibility & theming.** New UI must work in both light and dark themes, keep the monochrome identity (color only via course hues / status tokens), be keyboard-operable, and honor `prefers-reduced-motion`. +- **Isolate user text (bidi).** Course names, locations and titles come from the Technion catalog and are usually Hebrew (RTL); the UI around them is LTR English. Dropping one into app text unisolated lets the bidi algorithm reorder across the boundary and tear the app's own string apart. Render such values inside a ``, or — in plain-text strings like a `title` tooltip — wrap them with `isolate()` from `src/lib/bidi.ts`. - **Formatting is automated.** Don't hand-format; run `pnpm format`. ## Commits & PRs diff --git a/e2e/rtl-bidi.spec.ts b/e2e/rtl-bidi.spec.ts new file mode 100644 index 0000000..4746f7a --- /dev/null +++ b/e2e/rtl-bidi.spec.ts @@ -0,0 +1,77 @@ +import { test, expect, type Page } from '@playwright/test' + +// Technion course names are usually Hebrew (RTL); the UI around them is LTR English. +const HEBREW_COURSE = 'מערכות ספרתיות ומבנה המחשב' +const DAYS_OUT = 8 + +/** `yyyy-mm-dd`, `days` from today, so the due badge reads "d left". */ +function dueIn(days: number): string { + const d = new Date() + d.setDate(d.getDate() + days) + const mm = String(d.getMonth() + 1).padStart(2, '0') + const dd = String(d.getDate()).padStart(2, '0') + return `${d.getFullYear()}-${mm}-${dd}` +} + +async function seedHebrewCourseWithHomework(page: Page) { + await page.goto('/') + await page.getByRole('button', { name: 'Create your first semester' }).click() + await page.getByRole('button', { name: 'Create Semester' }).click() + + await page.getByRole('button', { name: 'Add Course' }).click() + await page.getByLabel('Course name').fill(HEBREW_COURSE) + await page.getByRole('button', { name: 'Save Course' }).click() + + await page.getByRole('button', { name: `Edit ${HEBREW_COURSE}` }).click() + await page.getByRole('tab', { name: 'Homework' }).click() + await page.getByLabel('Assignment', { exact: true }).fill('Wet 1') + await page.getByLabel('Due date for new assignment').fill(dueIn(DAYS_OUT)) + await page.getByRole('button', { name: 'Add assignment' }).click() + await page.keyboard.press('Escape') + + await expect(page.locator('#homework-list').getByText('Wet 1')).toBeVisible() +} + +/** + * Bidi reordering only happens in a real layout engine — jsdom has none, so this is + * the only place the fix can actually be observed. The sidebar subtitle is + * " · 8d left"; with the course name unisolated, the bidi algorithm + * dragged the badge's leading digit to the far side of the Hebrew run and rendered + * "8 · d left", tearing the badge into separate visual fragments. + */ +test.describe('RTL course names', () => { + test('the due badge stays one unbroken run to the right of a Hebrew course name', async ({ + page, + }) => { + await seedHebrewCourseWithHomework(page) + + // Located by position, not by , so the assertions below fail on the + // rendering if the isolation is ever dropped — not on a missing selector. + const subtitle = page.locator('#homework-list [data-homework-id] p').nth(1) + await expect(subtitle).toContainText(`${DAYS_OUT}d left`) + + const geom = await subtitle.evaluate((p: HTMLElement) => { + const badge = p.querySelector('span')! + // The name is a once isolated and a bare text node otherwise; a Range + // measures where it actually landed either way. + const name = p.querySelector('bdi') ?? p.firstChild! + const range = document.createRange() + range.selectNodeContents(name) + return { + // An inline box torn apart by bidi reordering yields one client rect per + // fragment, so the badge surviving as exactly one rect IS the fix. + badgeFragments: badge.getClientRects().length, + badgeLeft: badge.getBoundingClientRect().left, + nameRight: range.getBoundingClientRect().right, + lineDirection: getComputedStyle(p).direction, + } + }) + + // Unisolated, the badge splits into three fragments ("·", "8", "d left") and + // its leading digit lands left of the Hebrew name instead of after it. + expect(geom.badgeFragments).toBe(1) + expect(geom.badgeLeft).toBeGreaterThanOrEqual(geom.nameRight - 1) + // The Hebrew name resolves RTL inside its own run without flipping the line. + expect(geom.lineDirection).toBe('ltr') + }) +}) diff --git a/src/features/calendar/WeekCalendar.test.tsx b/src/features/calendar/WeekCalendar.test.tsx index ce47355..5693bc8 100644 --- a/src/features/calendar/WeekCalendar.test.tsx +++ b/src/features/calendar/WeekCalendar.test.tsx @@ -9,6 +9,11 @@ import { createCourse, type CourseInput } from '@/domain/course' // 2026-07-01 is a Wednesday. const NOW = new Date('2026-07-01T10:30:00') +const HEBREW_COURSE = 'מבוא למדעי המחשב' +// The Unicode isolates that fence an RTL run off from its LTR neighbors. +const FSI = '\u2068' +const PDI = '\u2069' + const input: CourseInput = { name: 'Algorithms 1', number: '', @@ -174,6 +179,41 @@ describe('WeekCalendar', () => { expect(screen.getByText('Wed')).toBeInTheDocument() }) + // Regression: next to a Hebrew course name the tooltip's time range was pulled + // across it and came back reversed — "12:00–10:00" for a 10:00–12:00 class. + it('isolates an RTL course name in the class-block tooltip so the time range keeps its order', () => { + setup((s) => + s.appStore.getState().addCourse(createCourse({ ...input, name: HEBREW_COURSE }, 'colorful')), + ) + const block = screen.getByRole('button', { name: /10:00.*12:00/ }) + expect(block).toHaveAttribute( + 'title', + `${FSI}${HEBREW_COURSE}${PDI} 10:00–12:00 · ${FSI}Taub 2${PDI}`, + ) + // Assistive tech reads logically, so the accessible name stays free of controls. + expect(block).toHaveAttribute('aria-label', `${HEBREW_COURSE} 10:00–12:00`) + }) + + it('isolates the RTL course name and title in an all-day chip tooltip', () => { + setup((s) => { + const course = createCourse({ ...input, name: HEBREW_COURSE }, 'colorful') + course.homework.push({ + id: 'h1', + title: 'Wet 1', + dueDate: '2026-07-03', + completed: false, + notes: '', + links: [], + }) + s.appStore.getState().addCourse(course) + }) + const chip = within(screen.getByTestId('all-day-row')).getByText(/Wet 1/) + expect(chip.closest('button')).toHaveAttribute( + 'title', + `${FSI}${HEBREW_COURSE}${PDI}: ${FSI}Wet 1${PDI}`, + ) + }) + it('collapses and expands the grid', async () => { const user = userEvent.setup() setup() diff --git a/src/features/calendar/WeekCalendar.tsx b/src/features/calendar/WeekCalendar.tsx index 42659c3..2d8dcea 100644 --- a/src/features/calendar/WeekCalendar.tsx +++ b/src/features/calendar/WeekCalendar.tsx @@ -16,6 +16,7 @@ import { useMediaQuery } from '@/hooks/useMediaQuery' import { IconButton } from '@/components/ui/IconButton' import { ChevronDownIcon } from '@/components/ui/icons' import { useCourseDialog } from '@/features/courses/CourseDialogProvider' +import { isolate } from '@/lib/bidi' import { cn } from '@/lib/cn' const DAY_NAMES = ['Sun', 'Mon', 'Tue', 'Wed', 'Thu', 'Fri', 'Sat'] @@ -189,7 +190,11 @@ export function WeekCalendar({ now: nowProp }: { now?: Date }) { key={`${slot.courseId}-${slot.day}-${i}`} type="button" onClick={() => openCourse({ courseId: slot.courseId })} - title={`${slot.courseName} ${slot.start}–${slot.end}${slot.location ? ` · ${slot.location}` : ''}`} + // A tooltip is plain text, so the Hebrew course name is fenced off with + // isolate() rather than — otherwise it swallows the time range and + // spits it back out reversed ("12:00–10:00"). The aria-label is read in + // logical order and needs no isolation. + title={`${isolate(slot.courseName)} ${slot.start}–${slot.end}${slot.location ? ` · ${isolate(slot.location)}` : ''}`} aria-label={`${slot.courseName} ${slot.start}–${slot.end}`} className="z-10 overflow-hidden rounded-control px-1 py-0.5 text-left text-[10px] leading-tight text-white/95 shadow-xs transition-[filter,box-shadow] duration-150 [text-shadow:0_1px_2px_rgba(0,0,0,0.35)] hover:z-20 hover:shadow-md hover:brightness-110 focus-visible:z-20 focus-visible:brightness-110 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-white/70 active:brightness-95" style={{ @@ -275,7 +280,11 @@ function AllDayRow({ backgroundColor: event.kind === 'exam' ? 'var(--error-bg)' : 'var(--success-bg)', textDecoration: event.completed ? 'line-through' : undefined, }} - title={event.kind === 'exam' ? event.title : `${event.courseName}: ${event.title}`} + title={ + event.kind === 'exam' + ? event.title + : `${isolate(event.courseName)}: ${isolate(event.title)}` + } > {event.kind === 'homework' ? ( {homework.title}

+ {/* Course names are Technion data and usually Hebrew. resolves the + name as its own directional run so the bidi algorithm can't drag the + badge's leading digit across it ("8 · d left"). See lib/bidi. */}

- {courseName} + {courseName} {badge ? · {badge.text} : null}

diff --git a/src/features/homework/HomeworkList.test.tsx b/src/features/homework/HomeworkList.test.tsx index 9e084ff..4358677 100644 --- a/src/features/homework/HomeworkList.test.tsx +++ b/src/features/homework/HomeworkList.test.tsx @@ -7,6 +7,7 @@ import { createMemoryStorage } from '@/services/storage/localStore' import { createCourse, type CourseInput } from '@/domain/course' const NOW = new Date('2026-07-04T12:00:00') +const HEBREW_COURSE = 'מערכות ספרתיות ומבנה המחשב' const baseInput: CourseInput = { name: 'Course', @@ -110,4 +111,20 @@ describe('HomeworkList', () => { const row = screen.getByText('Essay').closest('[data-homework-id]') expect(row).toHaveAttribute('data-overdue', 'true') }) + + // Regression: a Hebrew course name used to swallow the badge's leading digit and + // render the subtitle as "8 · d left" — the number torn off "d left". + it('isolates an RTL course name so the due badge cannot be reordered into it', () => { + setup((s) => { + const id = addCourse(s, HEBREW_COURSE) + s.appStore.getState().addHomework(id, 'Wet 1', '2026-07-12') // 8 days out + }) + + // The name resolves as its own bidi run, so the badge that follows it stays a + // whole, unreordered LTR string. + const name = screen.getByText(HEBREW_COURSE) + expect(name.tagName).toBe('BDI') + expect(getComputedStyle(name).unicodeBidi).toMatch(/isolate/) + expect(screen.getByText(/8d left/)).toBeInTheDocument() + }) }) diff --git a/src/lib/bidi.test.ts b/src/lib/bidi.test.ts new file mode 100644 index 0000000..1554087 --- /dev/null +++ b/src/lib/bidi.test.ts @@ -0,0 +1,24 @@ +import { isolate } from './bidi' + +const FSI = '\u2068' +const PDI = '\u2069' +const HEBREW = 'מבוא למדעי המחשב' + +describe('isolate', () => { + it('wraps a value in first-strong-isolate / pop-directional-isolate', () => { + expect(isolate(HEBREW)).toBe(`${FSI}${HEBREW}${PDI}`) + }) + + it('isolates LTR values too — the point is to fence off the neighbors, not the content', () => { + expect(isolate('Algorithms 1')).toBe(`${FSI}Algorithms 1${PDI}`) + }) + + it('passes an empty string through so no stray control characters are emitted', () => { + expect(isolate('')).toBe('') + }) + + it('leaves the visible text untouched', () => { + const wrapped = isolate(`${HEBREW} 10:00-12:00`) + expect(wrapped.replace(/[\u2068\u2069]/g, '')).toBe(`${HEBREW} 10:00-12:00`) + }) +}) diff --git a/src/lib/bidi.ts b/src/lib/bidi.ts new file mode 100644 index 0000000..afb6fac --- /dev/null +++ b/src/lib/bidi.ts @@ -0,0 +1,42 @@ +/** + * Bidirectional-text helpers. + * + * Tollab's chrome is LTR English (``, no `dir`), but course + * names, locations and assignment titles come from the Technion catalog and are + * usually Hebrew (RTL). When such a value is interpolated into app text, the + * Unicode Bidirectional Algorithm resolves the whole line as ONE paragraph and + * reorders across the boundary — so the app's own string gets torn apart + * (`[heb]` below stands in for a Hebrew course name): + * + * logical "[heb] · 8d left" rendered "8 · [heb] d left" + * logical "[heb] 10:00-12:00" rendered "12:00-10:00 [heb]" + * + * Digits are a *weak* bidi type: they inherit the direction of the text before + * them, so a number following Hebrew joins the RTL run and is carried to the far + * side of it — stranding whatever LTR text came after, and reversing a range + * that was split across the boundary. The fix is isolation: resolve the value on + * its own, then let it enter the surrounding paragraph as one neutral object. + * + * In JSX, isolate by rendering the value inside a `` element — the HTML UA + * stylesheet gives it `unicode-bidi: isolate` plus `dir="auto"`, and Tailwind's + * preflight leaves both alone. Reach for `isolate()` below only where markup is + * impossible, i.e. a plain-text string such as a `title` tooltip. + * + * `aria-label`s are deliberately NOT isolated: assistive tech consumes them in + * logical order and never runs the bidi algorithm, so the controls would be noise. + */ + +/** U+2068 FIRST STRONG ISOLATE — opens a run whose direction is auto-detected. */ +const FSI = '\u2068' + +/** U+2069 POP DIRECTIONAL ISOLATE — closes the innermost open isolate. */ +const PDI = '\u2069' + +/** + * Wraps text of unknown direction so it cannot reorder against its neighbors — + * the plain-text equivalent of ``. Empty input passes through untouched, so + * callers never emit a lone pair of invisible control characters. + */ +export function isolate(text: string): string { + return text === '' ? '' : FSI + text + PDI +}