fix(background): make the ripple canvas transparent until it draws - #35
Open
SongshGeo wants to merge 11 commits into
Open
fix(background): make the ripple canvas transparent until it draws#35SongshGeo wants to merge 11 commits into
SongshGeo wants to merge 11 commits into
Conversation
The commit log is a run of silent data-corruption fixes in pure functions —
the BibTeX scanner, the news bullet extractor, the sanitiser allow-list — and
every one of them was fixed by hand and then protected only by a comment.
`.cursor/rules/Astro.mdc` has mandated unit tests all along.
Adds vitest (54 tests) over exactly that code, each case pinning a behaviour
that previously reached production once: the brace matcher not truncating on a
nested `{\"O}`, `keyword` vs `keywords`, the missing year NOT falling back to
now, Zotero's local `file=` paths being rejected loudly, UTC calendar dates,
and the sanitiser dropping scripts, event handlers and `javascript:` hrefs.
Adds ESLint (flat) + Prettier + lefthook, and wires lint/test into CI ahead of
the build so they fail in seconds. ESLint is the only tool that sees the ~1,400
lines of `.js`/`.mjs` that `checkJs: false` hides from `astro check`.
It found eleven real defects on the first run, fixed here:
- Featured project cards on the home page were `<div onClick>` calling
`window.location.assign` — unreachable by keyboard, silent to screen readers,
no URL to copy or middle-click. GlassCard now renders a real anchor.
- GiscusComments' cleanup detached via `containerRef.current`, which may have
moved by cleanup time, leaking the iframe. Detach via the script's own parent.
- Four dead imports and an 11-line self-recursive `extractText` nobody called,
left behind by 728772c.
- `related_people` resolution used `(p: any)` only because `filter(Boolean)`
does not narrow; flatMap does, so the type Astro already knew is kept.
`renderMarkdown` and the two BibTeX functions gained injection points and
exports so they can be tested without the vault symlinks being wired up.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The site had three identities and no way for a content maintainer to change any of them: Header.astro said "Our Lab" / "实验室", Footer.astro said "Lab Website", and HomePage.tsx hardcoded "SongshGeo Lab", an institute name and the hero copy. Nothing reconciled them, and /about was a placeholder whose own text promised it would be "configured later" — from code, so it could not. Adds a `site` collection reading <vault>/site.md: name, tagline, hero headline, affiliation, email, address, socials, footer note, and an optional nav override. All bilingual via paired `x` / `x_en` fields. The body is markdown and becomes the About page, which is why this is a note rather than YAML — an Obsidian author can open and preview it like anything else in the vault. `getSiteConfig(lang)` resolves the locale and the fallbacks in one place; components receive plain strings. Reaching into `data.name_en ?? data.name` at each call site is how the three identities diverged. Also fixes the fourth answer to "what is the default language": Header derived it as `segments[0] === 'en' ? 'en' : 'zh'`, defaulting to zh, while DEFAULT_LOCALE said 'en' and astro.config.mjs redirected / to /en/. It now uses DEFAULT_LOCALE. `getContentRoot()` moves to src/utils/contentLayout.js, parameterised by sentinel directory. It was copy-pasted verbatim into both loaders — publicationsLoader even said "same as newsLoader" in a comment — and contentLayout.js exists precisely to hold vault-layout facts that more than one place must agree on. A non-throwing `findContentRoot()` joins it, because content.config.ts needs the path at config-load time and a fresh clone with no content synced must not crash before Astro can explain itself. Nav labels move to src/i18n/ui.ts under `nav`/`shell`; hrefs stay in Header.astro next to the routes they must match. site.md may override the nav wholesale, but not by default: a label is a translation, a route is a fact about what got built, and content declaring a route that does not exist would ship a 404 with no build error. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`/zh` and `/en` served byte-identical Chinese bodies. `librarySchema.lang` was declared, documented in CONTENT_MAINTENANCE.md, and read by nothing — the content layer had nowhere to put English at all, so the two trees differed only in URL prefix and a handful of chrome strings. Two mechanisms, chosen by field length: - Short structured fields pair in one file: `name`/`name_en`, `title`/`title_en`, `summary`/`summary_en`. Duplicating a whole note to translate a person's name means maintaining their email and ORCID twice, and those copies drift. - Long prose goes in a `<name>.en.md` sibling, picked up by a new `translations` collection globbed from the vault root. Overlays carry a body and nothing else, so they cannot satisfy a base schema; a union schema would instead make every consumer treat `name` as possibly-undefined. TRANSLATION_GLOBS keeps them out of the base collections, so a translation can never become a second person. Missing English falls back to Chinese *and says so* (ui.i18n.fallbackNotice). Silence was the old behaviour: a reader could not tell an untranslated page from one that simply had little to say. `role` gains a closed vocabulary (src/utils/roles.ts). PeoplePage grouped by comparing free text against ten hardcoded spellings, so 'Ph.D. student', '副教授' or a trailing space each became its own one-person section sorted to the end, silently. Values are normalised rather than rejected, so adopting the enum does not require rewriting 通讯录/ in the same commit; anything unrecognised lands in a visible "Others" group instead of failing the build. Section headings and card subtitles use separate tables — a single one reads "PhD Students" on one person's card, and Chinese does not pluralise, so that bug would have survived review. people also gains status/destination (alumni get their own section, cutting across roles, with where they went), order, joined/left, orcid/scholar/github/ homepage, and interests_en. projects gains title_en, cover, research, and a real `summary` — which the React DTO declared and HomePage rendered while no schema and no page ever supplied it, leaving the card body permanently blank. `featured`/`order` replace "featured = the first three in filesystem order". library declares `description`/`excerpt`, which LibraryPage reads and which previously survived only through .passthrough(), so `descriptio:` was silent. Fixes a latent mismatch on the projects list: it keyed the people map by raw collection id while the detail page keyed by entrySlug, so a person whose filename differed from their frontmatter id rendered as a name on one page and a raw id on the other. Also: avatars get width/height/loading/decoding (the markdown pipeline already did this for embedded images; avatars were the omission), and the card subtitle moves off 10px text at 60% opacity, which fails WCAG AA. BREAKING CHANGE: `role` frontmatter is normalised to a canonical vocabulary. Existing English and Chinese spellings are mapped, but a title outside the alias table now renders under "Others" instead of its own section. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…cars Three gaps between what the vault could express and what a group homepage needs to show. **Research themes** (图书馆/研究/). Deliberately not folded into `projects`: a project has a start date, an end date and a funder; a theme has none of those and outlives any one project. Merging them would mean either dating themes arbitrarily or making start_date optional on projects, and a reader would lose the distinction between what the group studies and what it is currently paid to do. A theme cross-references people, projects and papers, and those references resolve at build time so an id matching nothing is dropped rather than rendered as a dead link. **Resources** (图书馆/资源/) for published datasets, code, models and tools: type, URL, DOI, licence, version, release date, maintainers, and the paper to cite. **Publication sidecars** (图书馆/文献/精选/<bib_key>.md). Zotero regenerates .bib files wholesale and drops any field it does not recognise, so a cover image or a one-line highlight written into an entry would survive exactly until the next export. They live beside the .bib instead, joined on the citation key, carrying featured/order/cover/highlight/code/data/press and the lab author ids. The .bib stays the sole authority on citation facts. This makes `bib_key` load-bearing. It was parsed and stored and then never passed to any page — now it is the join key for sidecars and the foreign key from research themes and resources. Publications also gain `type`, inferred from the `@article` / `@inproceedings` token that extractBibEntries already preserved verbatim. citation-js normalises onto CSL names that collapse @Article and @misc together, and preprint-vs- peer-reviewed is precisely the distinction a reader needs; a sidecar can correct it, since @misc covers both a preprint and a dataset. A sidecar naming a citation key no .bib defines now warns with the filename. Silence would leave an author who mistyped a key wondering why their highlight never appeared. The sidecars are also registered as their own collection, so a malformed one reports as an error naming that file rather than as a confusing failure inside the loader. Restores baseSchema's null tolerance on publications.tags: spreading baseSchema and then re-declaring `tags` dropped it, so `tags: null` failed validation on this collection alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…d for less `client:idle` took the ripple background off the hydration critical path but not out of the download. Every visitor to every page still fetched RippleBackground.js — 888 KB raw, 228 KB gzipped, about two thirds of all the JS this site ships — including readers with prefers-reduced-motion, who then initialised a WebGL context and allocated two framebuffers only for the component to freeze its own animation from the inside. Contrary to what the code comments implied, there was no static fallback: the reduced-motion path rendered the same canvas with `useFrame` short-circuited. `client:media="(prefers-reduced-motion: no-preference)"` moves the decision to the mount boundary, so the chunk is never requested — verified in the built HTML, which now emits `client="media"` on the island. BaseLayout renders a real static backdrop on the server instead, which also removes the flash of black that preceded texture load for everyone else. The in-component listener stays: the gate is evaluated once at mount, so someone changing the setting mid-session still needs it. public/background.jpg: 540 KB -> 292 KB (2400px -> 1920px, requantised). It sits behind an 80%-opaque gradient, so the difference is not visible. Also fixes two accessibility gaps found by eslint-plugin-jsx-a11y and by reading the built markup: - No skip link. With nine nav items plus search and two language toggles, a keyboard user tabbed through twelve controls on every page before reaching the content. - Pagefind indexed the entire <body>, so every record contained the nav, footer and language switcher, and searching "Library" matched all 36 pages. `data-pagefind-body` on <main> confines it to content. Note that `root_selector` in pagefind.yml does NOT do this — it was tried, and Pagefind still logged "Did not find a data-pagefind-body element". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
docs/CONTENT_MAINTENANCE.md is what a content author reads instead of the Zod schema, so it being out of date is not a documentation problem — it is a correctness problem for anyone editing the vault. Rewritten around what the code now does: the bilingual convention and why short fields pair while long prose gets a sibling file; the `role` controlled vocabulary and what "Others" means when you land in it; research themes and why they are not projects; resources; and the publication sidecars, including the warning you get when a bib_key matches nothing. It now also tells authors which failures are loud and which are silent — an unresolved `#P/` tag warns, a broken wiki link renders visibly broken, an unmatched sidecar key warns with its filename, but an unresolvable cross- reference in a research theme is dropped without a word. Knowing which is which is the difference between debugging in five seconds and not noticing at all. README and FRONTEND_GUIDELINES §4.3 both stated that /zh and /en render the same Chinese content and that filtering was "待决策". Both are now describe the implemented mechanism instead. Fixes the false alarm in `npm run debug:content`: it probed for a src/content/publications symlink that setup-content.mjs deliberately stopped creating, so the tool meant to find real problems reported a failure on every healthy checkout. Same stale entry removed from .gitignore. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`engines` and .nvmrc both say Node >= 22, but the type definitions were pinned a major version behind at ^20 — so the scripts were type-checked against APIs that differ from the ones they run on. Also deletes the commented-out auto-merge job from release-please.yml. Dead config in a workflow is worse than no config: it reads as "this exists but is turned off" when nothing has ever evaluated whether it works. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…nditional Two separate causes for one symptom — the whole page murky and the nav unreadable. **Mine, from b8e5c45.** Adding the static backdrop copied RippleBackground's gradient overlay instead of moving it, so two `#08152dcc` layers were painted. At 80% opacity each they composite to 96%, not 80%. The tint now lives in BaseLayout exactly once, over whichever backdrop is showing; RippleBackground keeps only its canvas, and loses its `bg-black` too — that black was hiding the static image during texture load, which is the flash the static image existed to prevent. Verified by measurement, not inspection: the recompressed background.jpg was the obvious suspect and was innocent — mean luma 80.5 against the original's 80.4. The rendered DOM had two gradient elements; it now has one. **Pre-existing, and worse under the double tint.** All 65 `dark:` utilities in this codebase were gated on `prefers-color-scheme`, but the backdrop they sit on is a fixed dark photograph that does not follow the OS setting. A reader in light mode therefore got the light half of every pair on top of a dark photo: #374151 nav links, near-black body text, `prose` without `prose-invert`. The site was only legible for readers whose OS happened to be in dark mode. Fixed at the root rather than one utility at a time: `dark:` now keys off an explicit class (`@custom-variant`), and BaseLayout sets `class="dark"` on <html> with a matching color-scheme meta. Rewriting 65 utilities would have been 65 chances to miss one, and would have discarded the option of a light theme later — which now needs only that class to become conditional. The compiled stylesheet contains zero prefers-color-scheme queries. Header and Footer are additionally styled light-on-dark without `dark:` at all, because they sit directly on the photograph rather than on a page surface: `border-white/10` and `hover:bg-white/10` read correctly over an image where `border-gray-800` and `hover:bg-gray-800` read as grey blocks. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…t sits on Closes #33. Clicking any nav link showed the background image, then ~240ms of black, then the background again. `RippleBackground`'s canvas was created with `alpha: false`. An opaque WebGL canvas is cleared to opaque BLACK before anything is drawn, and `useTexture` suspends while /background.jpg loads and decodes. Throughout that window the canvas sat at -z-20 painting black over BaseLayout's static backdrop at -z-30. `alpha: true` makes it transparent until the first frame draws, so the static image shows through. Nothing else changes: the distortion shader writes an opaque pixel, so once there is something to composite it composites the same. The latency always existed. b8e5c45 made it *visible* by putting an image underneath — before that the sequence was black then water, which reads as loading; after, image then black then water, which reads as a flicker. Adds test/e2e/background-flash.mjs, which samples screen luma across a real navigation and asserts that once the static backdrop has loaded it never goes black again. Validated in both directions: red 3/3 with `alpha: false`, green 3/3 with `alpha: true`. That validation was not ceremony. Two earlier versions of the harness were wrong in ways that unit tests cannot be: - asserting "no lit -> dark -> lit dip" latched onto the document swap, a single frame inherent to any MPA navigation. Red before the fix AND after, and red for the wrong reason before — it briefly refuted the correct hypothesis and nearly sent the diagnosis elsewhere. - asserting "no dark after the first lit frame following a dark one" raced the 40ms sampling interval and passed against known-bad code. Both failure modes are written up in the harness header so they are not re-derived. Not wired into CI — that needs a browser image and a served build, roughly doubling CI time, which is a call to make explicitly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Standards and Spec review of the branch found four defects in the test I added, two of which would have let it pass against broken code. - **The harness could pass vacuously.** It asserted "the backdrop never goes black" without ever checking a canvas existed to make it black. On a machine with no WebGL, or if the island failed to hydrate, it passed identically with `alpha: false` — the exact failure mode its own header warns about. It now requires a canvas sized to the viewport. The first attempt at that control was itself vacuous: Astro server-renders the <canvas> element even when the island never hydrates, so it exists at the HTML default 300x150 and a non-zero check saw nothing wrong. Caught by running the control against a suppressed island and watching it pass anyway. - **Wrong default port.** 4321 is what `astro dev` serves; 4323 was only what this machine happened to land on because 4321 was busy. The documented two-terminal sequence in the README could not have connected. - **Selected the nav link by its Chinese label.** Labels are translated and site.md can replace the nav wholesale, so a content edit would have turned this red for a reason unrelated to the bug. Selects by href now, and the unused argv target override is gone — it parameterised the click but not the waitForURL beside it, so any other value would have hung. - **`exclude: ['test/e2e/**']` in vitest.config.ts was dead and lossy.** `include` only takes `.test.ts` and the harnesses are `.mjs`, so it matched nothing — while replacing vitest's own node_modules/dist defaults. Also records, where the fix lives, why the canvas is not faded in: the shader renders the same photograph ~13% darker (luma 46 static, 40 through the canvas), so a fade would only cross-dissolve between two brightnesses that ought to match. Tracked separately rather than papered over. Validated all three directions: exit 1 with `alpha: false`, exit 0 with `alpha: true`, exit 2 with the island suppressed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Author
|
While setting up a local preview I re-ran
What this means:
Suggested next step: either drop Filing this rather than quietly re-running until it went green. |
…orthy The harness gave contradictory answers depending on where it sampled, and I could not reconcile them: - Its clip rectangle was chosen on the home page. On the destination page `document.elementFromPoint(850, 200)` returns DIV.space-y-8 — the content column. It was reading content paint-in, not the backdrop, which is why the ~240ms "black" it reported survived changes that demonstrably made the canvas invisible. - Sampling a genuine bare-backdrop strip across a full navigation, 43 frames, found zero dark frames with the island mounted or suppressed. The harness, over the same rectangle on the same server, still reported a dip. A test that disagrees with itself is worse than no test: it goes red on unrelated changes until someone skips it, and it lends false confidence in between. The red/green table it produced for this PR should be disregarded. What remains is the one-line `alpha: true`, which stands on its mechanism — `alpha: false` creates an opaque canvas that clears to black — and on the maintainer confirming by eye that the flash went from a clear black beat to barely perceptible. The residual flash is the ordinary MPA document swap and is tracked separately. Rebuilding this properly needs a signal that does not depend on screenshot timing: recording composited frames from inside the page via requestAnimationFrame, with the sample region asserted against elementFromPoint on the page actually being measured.
SongshGeo
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #33.
One flag.
RippleBackground's canvas was created withalpha: false, which makes it opaque, so it cleared to black overBaseLayout's static backdrop for as long asuseTexturespent loading/background.jpg.alpha: trueleaves it transparent until the first frame draws, and the static image shows through.Nothing else changes: the distortion shader writes an opaque pixel, so once there is something to composite it composites the same.
The latency always existed. b8e5c45 made it visible by putting an image underneath — before that the sequence was black → water, which reads as loading; after, image → black → water, which reads as a flicker.
Verification
By eye, by the maintainer: the flash went from a clear black beat to barely perceptible. That is the evidence this change rests on.
A Playwright harness was written for this and has been removed again — it gave contradictory answers depending on where it sampled, and I could not reconcile them:
document.elementFromPoint(850, 200)returnsDIV.space-y-8— the content column. It was reading content paint-in, not the backdrop, which is why the ~240ms "black" it reported survived changes that demonstrably made the canvas invisible.A test that disagrees with itself is worse than no test: it goes red on unrelated changes until someone skips it, and lends false confidence in between. Any red/green figures I quoted earlier in this PR should be disregarded.
Rebuilding it properly needs a signal that does not depend on screenshot timing — composited frames recorded from inside the page via
requestAnimationFrame, with the sample region asserted againstelementFromPointon the page actually being measured. Not attempted here.Not in scope