diff --git a/android/app/src/main/java/com/fabledsword/thoughtsync/ui/DerivedTint.kt b/android/app/src/main/java/com/fabledsword/thoughtsync/ui/DerivedTint.kt new file mode 100644 index 0000000..1ca4ede --- /dev/null +++ b/android/app/src/main/java/com/fabledsword/thoughtsync/ui/DerivedTint.kt @@ -0,0 +1,93 @@ +package com.fabledsword.thoughtsync.ui + +// The colour a note has when nothing chose one for it. +// +// A board of `default` notes is a wall of white rectangles and the eye gets no help +// telling one from the next. Every note now carries some tint; this is where an +// untagged one gets it. +// +// "RANDOM" MEANS DERIVED. The operator asked for "random subdued colors", but a tint +// rolled at render time would differ between the phone and the browser and change on +// every reload. Hashing the note's id is deterministic, identical on every surface, +// costs no column and no migration, and a note keeps its colour for life — which is +// what "random" actually meant here. +// +// THIS IS HALF A MIRRORED PAIR. `frontend/src/notes/colors.ts` computes the same hash +// over the same key order, and the two must agree exactly or a note is one colour on +// the phone and another in the browser. Same discipline as the checklist grammar's +// three implementations, and the same reason: a value that disagrees across surfaces +// is a bug you cannot unsee and cannot explain. +// +// NO COMPOSE IN THIS FILE, deliberately. It is the half of the pair that CAN be +// pinned by a host-JVM test, and staying free of `androidx.compose` is what keeps +// `DerivedTintTest` runnable in the Unit tests step rather than on an emulator. The +// web side has no test runner at all, so this test is the only mechanical guard the +// mirror gets — see the fixture comment in colors.ts. + +/** + * The tints a derived colour can land on: `NOTE_TINTS`' keys minus `default`, which + * is the white this exists to eliminate. `gray` stays — `bg-neutral-100` reads as a + * deliberate card against the board's `bg-neutral-50`, not as an absence. + * + * Order is load-bearing and matches `DERIVED_TINT_KEYS` in colors.ts. Reordering + * this list silently recolours every untagged note on one surface only. + */ +val DERIVED_TINT_KEYS: List = + listOf("red", "orange", "yellow", "green", "teal", "blue", "purple", "pink", "gray") + +private const val FNV_OFFSET_BASIS = -0x7ee3623b // 0x811c9dc5 as a signed Int +private const val FNV_PRIME = 0x01000193 +private const val BYTE_MASK = 0xFF +private const val UNSIGNED_MASK = 0xFFFFFFFFL + +/** + * FNV-1a over the id's bytes, 32-bit. + * + * Chosen because both languages compute it identically in ten lines with no library. + * Explicitly NOT `String.hashCode()`: Kotlin's is specified but JS has no equivalent, + * and reimplementing Java's from memory in TypeScript is exactly how a mirror drifts. + * + * `and BYTE_MASK` is a no-op for the ASCII of a UUID, and is kept because it states + * the intent — this hashes BYTES, so the TypeScript side reading `charCodeAt(i) & + * 0xff` is the same function rather than a coincidence. + * + * Overflow is the point: Kotlin's `Int` wraps on multiply, which is what the web's + * `Math.imul` exists to reproduce. + */ +fun tintHash(id: String): Int { + var hash = FNV_OFFSET_BASIS + for (ch in id) { + hash = hash xor (ch.code and BYTE_MASK) + hash *= FNV_PRIME + } + return hash +} + +/** The tint a note with no colour of its own wears. Stable for the life of the note. */ +fun derivedTint(id: String): String { + // Through Long to read the hash as unsigned. A signed remainder would be negative + // for half of all ids and index out of the list. + val index = (tintHash(id).toLong() and UNSIGNED_MASK) % DERIVED_TINT_KEYS.size + return DERIVED_TINT_KEYS[index.toInt()] +} + +/** + * The colour key to actually paint a note with. + * + * An explicitly-picked colour still wins — the picker is on its way out (milestone + * 309 step 5) but it has not gone yet, and a note the operator coloured by hand + * changing under them would read as data loss. + * + * `known` is passed in rather than read from `NOTE_TINTS` so this file stays free of + * Compose and therefore testable; `noteTintFor` supplies the real set. + */ +fun resolvedNoteColor(id: String, color: String, known: Set): String = + when { + color.isNotEmpty() && color != "default" && color in known -> color + // A draft carries DRAFT_ID (""), so there is no identity to derive from yet. + // Staying white until the note exists costs one colour change at save time; + // hashing the empty string instead would give EVERY draft the same tint and + // then change it anyway, which is two surprises where one will do. + id.isEmpty() -> "default" + else -> derivedTint(id) + } diff --git a/android/app/src/main/java/com/fabledsword/thoughtsync/ui/NoteCard.kt b/android/app/src/main/java/com/fabledsword/thoughtsync/ui/NoteCard.kt index a0ea671..b6286b7 100644 --- a/android/app/src/main/java/com/fabledsword/thoughtsync/ui/NoteCard.kt +++ b/android/app/src/main/java/com/fabledsword/thoughtsync/ui/NoteCard.kt @@ -37,7 +37,7 @@ fun NoteCard( onToggleItem: (Int, Boolean) -> Unit, ) { val dark = isSystemInDarkTheme() - val tint = noteTint(note.color) + val tint = noteTintFor(note.id, note.color) Column( modifier = diff --git a/android/app/src/main/java/com/fabledsword/thoughtsync/ui/NoteEditorScreen.kt b/android/app/src/main/java/com/fabledsword/thoughtsync/ui/NoteEditorScreen.kt index cc9d16c..d116fb3 100644 --- a/android/app/src/main/java/com/fabledsword/thoughtsync/ui/NoteEditorScreen.kt +++ b/android/app/src/main/java/com/fabledsword/thoughtsync/ui/NoteEditorScreen.kt @@ -63,7 +63,7 @@ fun NoteEditorScreen( onAction: (EditorAction) -> Unit, ) { val dark = isSystemInDarkTheme() - val tint = noteTint(note.color) + val tint = noteTintFor(note.id, note.color) // Keyed by the SESSION, not by note.id: the editor is reused across notes, so it // needs a key — but a draft's id changes the moment it is first saved, and diff --git a/android/app/src/main/java/com/fabledsword/thoughtsync/ui/NoteTint.kt b/android/app/src/main/java/com/fabledsword/thoughtsync/ui/NoteTint.kt index 3a08d6d..aef2744 100644 --- a/android/app/src/main/java/com/fabledsword/thoughtsync/ui/NoteTint.kt +++ b/android/app/src/main/java/com/fabledsword/thoughtsync/ui/NoteTint.kt @@ -175,3 +175,15 @@ val NOTE_TINTS: Map = @Composable @ReadOnlyComposable fun noteTint(key: String): NoteTint = NOTE_TINTS[key] ?: NOTE_TINTS.getValue("default") + +/** + * The tint for a NOTE, which is not the same thing as looking up its stored key: a + * note that has no colour of its own gets one derived from its id, so that a board + * of untagged notes reads as individual items rather than a wall of white. + * + * See `DerivedTint.kt` — the rule lives there, free of Compose, so it can be tested. + */ +@Composable +@ReadOnlyComposable +fun noteTintFor(id: String, color: String): NoteTint = + noteTint(resolvedNoteColor(id, color, NOTE_TINTS.keys)) diff --git a/android/app/src/test/java/com/fabledsword/thoughtsync/ui/DerivedTintTest.kt b/android/app/src/test/java/com/fabledsword/thoughtsync/ui/DerivedTintTest.kt new file mode 100644 index 0000000..8a34fe5 --- /dev/null +++ b/android/app/src/test/java/com/fabledsword/thoughtsync/ui/DerivedTintTest.kt @@ -0,0 +1,102 @@ +package com.fabledsword.thoughtsync.ui + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotEquals +import org.junit.Test + +/** + * Pins the derived-tint rule against `frontend/src/notes/colors.ts`. + * + * These are not tests of Kotlin — they are the ONE mechanical guard the mirrored pair + * has. The web side is TypeScript with no test runner (its CI lane is `vue-tsc + * --noEmit` and nothing else), so if these values drift, nothing on that surface will + * say so and a note will simply be a different colour on the phone than in the + * browser. The same four ids and hashes are written into colors.ts as a comment; + * changing either side means changing both and re-checking here. + */ +class DerivedTintTest { + @Test + fun `hashes match the fixture shared with the web`() { + // Kotlin's Int is signed, so the two hashes above 0x7FFFFFFF are written as + // their negative literal. The unsigned value in the comment is what colors.ts + // records and what an implementation of FNV-1a will actually produce. + assertEquals(-0x41B8712F, tintHash("00000000-0000-0000-0000-000000000000")) // 0xbe478ed1 + assertEquals(0x3D75CC01, tintHash("11111111-1111-1111-1111-111111111111")) + assertEquals(-0x0EF71AD0, tintHash("6ba7b810-9dad-11d1-80b4-00c04fd430c8")) // 0xf108e530 + assertEquals(0x5B651540, tintHash("f47ac10b-58cc-4372-a567-0e02b2c3d479")) + } + + @Test + fun `tints match the fixture shared with the web`() { + assertEquals("purple", derivedTint("00000000-0000-0000-0000-000000000000")) + assertEquals("blue", derivedTint("11111111-1111-1111-1111-111111111111")) + assertEquals("orange", derivedTint("6ba7b810-9dad-11d1-80b4-00c04fd430c8")) + assertEquals("orange", derivedTint("f47ac10b-58cc-4372-a567-0e02b2c3d479")) + } + + /** Half of all 32-bit hashes are negative as Kotlin Ints; a signed remainder would + * index out of the list for those. The bug this catches is a crash, not a wrong + * colour, so it is worth more than one id's worth of coverage. */ + @Test + fun `every tint is a real palette key, over many ids`() { + for (n in 0 until 2000) { + val tint = derivedTint("note-$n") + assertEquals(true, tint in DERIVED_TINT_KEYS) + } + } + + @Test + fun `the derived palette excludes default`() { + assertEquals(false, "default" in DERIVED_TINT_KEYS) + assertEquals(9, DERIVED_TINT_KEYS.size) + } + + /** The order IS the mapping — reordering silently recolours every untagged note + * on one surface only. Written out longhand so a reorder fails here loudly. */ + @Test + fun `key order matches colors ts`() { + assertEquals( + listOf("red", "orange", "yellow", "green", "teal", "blue", "purple", "pink", "gray"), + DERIVED_TINT_KEYS, + ) + } + + @Test + fun `an explicitly picked colour still wins`() { + val known = DERIVED_TINT_KEYS.toSet() + "default" + assertEquals("teal", resolvedNoteColor("any-id", "teal", known)) + } + + @Test + fun `an unknown colour key falls back to the derived tint`() { + val known = DERIVED_TINT_KEYS.toSet() + "default" + val id = "00000000-0000-0000-0000-000000000000" + assertEquals("purple", resolvedNoteColor(id, "chartreuse", known)) + } + + /** `default` is not a choice, it is the absence of one — so a note stored as + * `default` gets a derived tint rather than staying white. That is the whole + * point of the change. */ + @Test + fun `a default colour is treated as no colour`() { + val known = DERIVED_TINT_KEYS.toSet() + "default" + val id = "11111111-1111-1111-1111-111111111111" + assertEquals("blue", resolvedNoteColor(id, "default", known)) + assertNotEquals("default", resolvedNoteColor(id, "default", known)) + } + + /** A draft has no id yet. It must not be hashed — see the comment in DerivedTint. */ + @Test + fun `a draft stays default until it has an id`() { + val known = DERIVED_TINT_KEYS.toSet() + "default" + assertEquals("default", resolvedNoteColor("", "", known)) + assertEquals("default", resolvedNoteColor("", "default", known)) + } + + /** The reason the feature exists: two adjacent notes should not look identical. */ + @Test + fun `the tint spreads across the palette`() { + val seen = (0 until 500).map { derivedTint("spread-$it") }.toSet() + assertEquals(DERIVED_TINT_KEYS.size, seen.size) + } +} diff --git a/frontend/src/components/NoteCard.vue b/frontend/src/components/NoteCard.vue index 58fff4b..5a193c9 100644 --- a/frontend/src/components/NoteCard.vue +++ b/frontend/src/components/NoteCard.vue @@ -7,6 +7,7 @@ import { NOTE_COLOR_KEYS, NOTE_COLOR_LABELS, NOTE_SWATCH_CLASSES, + resolveNoteColor, type NoteColor, } from "../notes/colors"; import type { Note } from "../stores/notes"; @@ -189,8 +190,10 @@ async function snoozeReminder(minutes: number): Promise { emit("reminder-changed"); } -function cardClass(color: NoteColor): string { - return NOTE_CARD_CLASSES[color] ?? NOTE_CARD_CLASSES.default; +// Takes the note, not its colour: a note with no colour of its own gets one +// derived from its id, so the card can no longer be painted from a single field. +function cardClass(note: Note): string { + return NOTE_CARD_CLASSES[resolveNoteColor(note)] ?? NOTE_CARD_CLASSES.default; } function labelChip(color: string): string { @@ -225,7 +228,7 @@ onBeforeUnmount(() => document.removeEventListener("mousedown", onDocMousedown)) ref="root" class="group relative mb-4 break-inside-avoid rounded-xl border p-3 shadow-sm transition hover:shadow-md" :class="[ - cardClass(note.color), + cardClass(note), dragging ? 'opacity-40' : '', dragOver ? 'scale-[1.02] shadow-lg ring-2 ring-brand ring-offset-2 ring-offset-white dark:ring-offset-neutral-950' diff --git a/frontend/src/notes/colors.ts b/frontend/src/notes/colors.ts index 95765f5..8e9af64 100644 --- a/frontend/src/notes/colors.ts +++ b/frontend/src/notes/colors.ts @@ -84,3 +84,87 @@ export const NOTE_COLOR_LABELS: Record = { pink: "Pink", gray: "Gray", }; + +// --------------------------------------------------------------------------- +// Derived tints — the colour a note has when nothing chose one for it. +// +// A board of `default` notes is a wall of white rectangles and the eye gets no +// help telling one from the next. Every note now carries some tint; this is where +// an untagged one gets it. +// +// "RANDOM" MEANS DERIVED. The operator asked for "random subdued colors", but a +// tint rolled at render time would differ between the phone and the browser and +// change on every reload. Hashing the note's id is deterministic, identical on +// every surface, costs no column and no migration, and a note keeps its colour +// for life — which is what "random" actually meant here. +// +// THIS IS HALF A MIRRORED PAIR. `android/.../ui/NoteTint.kt` computes the same +// hash over the same key order, and the two must agree exactly or a note is one +// colour on the phone and another in the browser. Same discipline as the +// checklist grammar's three implementations, and the same reason: a value that +// disagrees across surfaces is a bug you cannot unsee and cannot explain. +// +// The Kotlin side has a unit test pinning the fixture below. THIS SIDE HAS NO +// MECHANICAL GUARD — the frontend has no test runner, only `vue-tsc --noEmit`. +// If you change anything here, check it against the fixture by hand. + +/** The tints a derived colour can land on: the palette minus `default`, which is + * the white this exists to eliminate. `gray` stays — `bg-neutral-100` reads as a + * deliberate card against the board's `bg-neutral-50`, not as an absence. */ +export const DERIVED_TINT_KEYS: readonly NoteColor[] = NOTE_COLOR_KEYS.filter( + (key) => key !== "default", +); + +/** + * FNV-1a over the id's bytes, 32-bit. + * + * Chosen because both languages compute it identically in ten lines with no + * library. Explicitly NOT `String.hashCode()`: Kotlin's is specified but JS has + * no equivalent, and reimplementing Java's from memory in TypeScript is exactly + * how a mirror drifts. + * + * `& 0xff` is a no-op for the ASCII of a UUID, and is kept because it states the + * intent — this hashes BYTES, so the Kotlin side reading `id[i].code and 0xFF` + * is the same function rather than a coincidence. + */ +export function tintHash(id: string): number { + let hash = 0x811c9dc5; + for (let i = 0; i < id.length; i++) { + hash ^= id.charCodeAt(i) & 0xff; + // Math.imul, not `*`: JS numbers are doubles and a 32-bit overflow would be + // silently kept as precision instead of wrapping the way Kotlin's Int does. + hash = Math.imul(hash, 0x01000193) >>> 0; + } + return hash >>> 0; +} + +/** The tint a note with no colour of its own wears. Stable for the life of the note. */ +export function derivedTint(id: string): NoteColor { + return DERIVED_TINT_KEYS[tintHash(id) % DERIVED_TINT_KEYS.length]; +} + +/** + * The colour to actually paint a note. + * + * An explicitly-picked colour still wins — the picker is on its way out + * (milestone 309 step 5) but it has not gone yet, and a note the operator + * coloured by hand changing under them would read as data loss. + */ +export function resolveNoteColor(note: { id: string; color?: string | null }): NoteColor { + const picked = note.color as NoteColor | undefined | null; + if (picked && picked !== "default" && picked in NOTE_CARD_CLASSES) return picked; + // No id yet means an unsaved draft: nothing to derive from. Staying white until the + // note exists costs one colour change at save time; hashing the empty string would + // give every draft the same tint and then change it anyway. + if (!note.id) return "default"; + return derivedTint(note.id); +} + +// Fixture — the same ids and expected keys the Kotlin test asserts. Kept here as +// prose because there is nowhere on this side to assert it. If you change the hash +// or the key order, these four must still hold on BOTH surfaces: +// +// 00000000-0000-0000-0000-000000000000 0xbe478ed1 purple +// 11111111-1111-1111-1111-111111111111 0x3d75cc01 blue +// 6ba7b810-9dad-11d1-80b4-00c04fd430c8 0xf108e530 orange +// f47ac10b-58cc-4372-a567-0e02b2c3d479 0x5b651540 orange