Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<bdi>`, 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
Expand Down
77 changes: 77 additions & 0 deletions e2e/rtl-bidi.spec.ts
Original file line number Diff line number Diff line change
@@ -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 "<days>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
* "<hebrew course> · 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 · <hebrew course> 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 <bdi>, 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 <bdi> 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')
})
})
40 changes: 40 additions & 0 deletions src/features/calendar/WeekCalendar.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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: '',
Expand Down Expand Up @@ -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()
Expand Down
13 changes: 11 additions & 2 deletions src/features/calendar/WeekCalendar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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']
Expand Down Expand Up @@ -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 <bdi> — 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={{
Expand Down Expand Up @@ -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' ? (
<span
Expand Down
5 changes: 4 additions & 1 deletion src/features/homework/HomeworkItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,11 @@ function SidebarRow({
>
{homework.title}
</p>
{/* Course names are Technion data and usually Hebrew. <bdi> resolves the
name as its own directional run so the bidi algorithm can't drag the
badge's leading digit across it ("8 · <hebrew> d left"). See lib/bidi. */}
<p className="truncate text-xs text-ink-muted">
{courseName}
<bdi>{courseName}</bdi>
{badge ? <span className={cn('ml-1.5', badge.tone)}>· {badge.text}</span> : null}
</p>
</div>
Expand Down
17 changes: 17 additions & 0 deletions src/features/homework/HomeworkList.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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 · <hebrew> 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()
})
})
24 changes: 24 additions & 0 deletions src/lib/bidi.test.ts
Original file line number Diff line number Diff line change
@@ -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`)
})
})
42 changes: 42 additions & 0 deletions src/lib/bidi.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/**
* Bidirectional-text helpers.
*
* Tollab's chrome is LTR English (`<html lang="en">`, 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 `<bdi>` 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 `<bdi>`. 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
}
Loading