From e478e9b4c2d148ce8a5dd747f5550137aa1f5d02 Mon Sep 17 00:00:00 2001 From: ssavutu Date: Thu, 3 Sep 2026 11:32:28 -0400 Subject: [PATCH 1/2] Say it once, in the fewest words that still say it The comments in this codebase had drifted into essays. A doc comment would state the rule, then argue for it, then restate the argument in different words, on the assumption that the reader needs convincing. They don't: the people reading this are the people who write it. Trim every comment to the claim plus whatever a future editor would get wrong without it. The kept rationale is the load-bearing kind -- why INTERVAL 0 HOUR cannot stand in for "no limit", why a blank banner text is read raw, why the response is re-resolved rather than echoed. What goes is the newsroom-workflow justification and the second pass at a point already made. Also replace the double hyphen everywhere it stood in for an em dash, in comments, test failure messages, docstrings and config. A comma, colon, semicolon or full stop always fits, and usually reads better: the dash was doing the work of not deciding which one was meant. Left alone where `--` is syntax rather than punctuation (shell flags, SQL line comments, the imported WordPress article bodies in init_schema.sql). Two user-facing blurbs shortened along the way, for the same reason: the "Breaking news" checkbox and the Settings section that describes it. No behaviour change. Build, vet, go test, tsc, vitest and the deploy script suite all pass. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01L4qBhBdQto1yNp1zP7VLYc --- .github/workflows/ci.yml | 10 +-- .github/workflows/publish.yml | 2 +- .gitignore | 2 +- deploy/cms.env.example | 6 +- deploy/compose.cms.yml | 8 +- deploy/scripts/common.sh | 2 +- deploy/scripts/deploy.sh | 4 +- deploy/scripts/deploy_scripts_test.sh | 2 +- docker-compose.yml | 4 +- embeddings/app.py | 8 +- frontend/nginx.conf | 2 +- frontend/src/auth/adminOnlyNoticeContext.ts | 2 +- frontend/src/auth/urls.ts | 2 +- frontend/src/components/MediaPicker.tsx | 6 +- frontend/src/components/TrixEditor.css | 12 +-- frontend/src/components/TrixEditor.tsx | 74 +++++++++--------- frontend/src/components/trixImageHtml.ts | 12 +-- frontend/src/components/ui/datetime-field.tsx | 6 +- frontend/src/lib/clipboard.ts | 2 +- frontend/src/pages/editArticleView.test.tsx | 6 +- frontend/src/pages/editArticleView.tsx | 15 ++-- frontend/src/pages/mediaView.tsx | 4 +- frontend/src/pages/pollView.tsx | 10 +-- frontend/src/pages/sectionsView.tsx | 10 +-- frontend/src/pages/settingsPage.tsx | 28 +++---- frontend/src/trix.d.ts | 6 +- .../backfill_gallery_flags.cpython-314.pyc | Bin 0 -> 17707 bytes .../backfill_parent_sections.cpython-314.pyc | Bin 0 -> 25085 bytes .../build_database.cpython-314.pyc | Bin 0 -> 24336 bytes .../first_time_setup.cpython-314.pyc | Bin 0 -> 15177 bytes .../generate_wordpress_sql.cpython-314.pyc | Bin 0 -> 20177 bytes .../report_duplicate_slugs.cpython-314.pyc | Bin 0 -> 19433 bytes .../reseed_from_etl.cpython-314.pyc | Bin 0 -> 30340 bytes .../setup_containers.cpython-314.pyc | Bin 0 -> 4336 bytes scripts/backfill_gallery_flags.py | 2 +- scripts/backfill_parent_sections.py | 10 +-- scripts/build_database.py | 6 +- scripts/generate_wordpress_sql.py | 4 +- scripts/report_duplicate_slugs.py | 6 +- scripts/reseed_from_etl.py | 14 ++-- server/cmd/backfill-excerpts/main.go | 2 +- .../internal/database/article_categories.go | 10 +-- .../internal/database/article_embeddings.go | 2 +- .../article_embeddings_integration_test.go | 8 +- server/internal/database/article_search.go | 8 +- .../database/article_search_fusion_test.go | 8 +- .../article_search_integration_test.go | 2 +- .../breaking_news_integration_test.go | 32 +++----- server/internal/database/database.go | 8 +- server/internal/database/featured_article.go | 2 +- server/internal/database/footer_settings.go | 16 ++-- .../database/homepage_carousel_settings.go | 6 +- server/internal/database/http_models.go | 30 +++---- server/internal/database/http_models_test.go | 4 +- server/internal/database/media.go | 6 +- server/internal/database/polls.go | 10 +-- .../database/polls_integration_test.go | 2 +- .../database/schema/site_taxonomy.sql | 2 +- server/internal/database/seo.go | 10 +-- server/internal/database/seo_test.go | 4 +- server/internal/database/settings.go | 57 +++++--------- server/internal/database/settings_cache.go | 6 +- .../internal/database/settings_cache_test.go | 4 +- server/internal/database/tags.go | 12 +-- server/internal/database/tags_test.go | 2 +- server/internal/database/taxonomy.go | 68 ++++++++-------- .../database/taxonomy_integration_test.go | 26 +++--- server/internal/database/taxonomy_test.go | 8 +- server/internal/database/users.go | 24 +++--- server/internal/embeddings/client.go | 4 +- server/internal/embeddings/reconciler.go | 6 +- .../embeddings/reconciler_integration_test.go | 8 +- .../article_patch_integration_test.go | 14 ++-- ...breaking_news_settings_integration_test.go | 12 +-- .../featured_article_integration_test.go | 6 +- server/internal/handlers/handlers.go | 44 +++++------ server/internal/handlers/handlers_test.go | 4 +- server/internal/handlers/media.go | 22 +++--- server/internal/handlers/media_fetch_test.go | 2 +- .../handlers/media_integration_test.go | 4 +- server/internal/handlers/media_test.go | 6 +- server/internal/handlers/poll_handlers.go | 4 +- .../internal/handlers/poll_handlers_test.go | 4 +- server/internal/handlers/settings.go | 15 ++-- .../handlers/tags_integration_test.go | 6 +- server/internal/handlers/taxonomy.go | 14 ++-- .../handlers/taxonomy_integration_test.go | 36 ++++----- server/internal/handlers/taxonomy_test.go | 2 +- server/internal/middleware/compression.go | 10 +-- server/internal/models/api_responses.go | 37 +++------ server/internal/routes/routes.go | 6 +- server/main.go | 2 +- 92 files changed, 440 insertions(+), 494 deletions(-) create mode 100644 scripts/__pycache__/backfill_gallery_flags.cpython-314.pyc create mode 100644 scripts/__pycache__/backfill_parent_sections.cpython-314.pyc create mode 100644 scripts/__pycache__/build_database.cpython-314.pyc create mode 100644 scripts/__pycache__/first_time_setup.cpython-314.pyc create mode 100644 scripts/__pycache__/generate_wordpress_sql.cpython-314.pyc create mode 100644 scripts/__pycache__/report_duplicate_slugs.cpython-314.pyc create mode 100644 scripts/__pycache__/reseed_from_etl.cpython-314.pyc create mode 100644 scripts/__pycache__/setup_containers.cpython-314.pyc diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 03c10d8..da47d69 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -57,7 +57,7 @@ jobs: run: npm run build # The Docker build regenerates the Swagger docs from the handler annotations, - # so the binary always serves a current spec -- but server/docs is also + # so the binary always serves a current spec, but server/docs is also # committed, and the Pages site publishes that committed copy. Nothing forced # the two to agree, so annotations could change without a follow-up # `swag init` and the published reference would drift from the real API with @@ -121,8 +121,8 @@ jobs: run: deploy/scripts/deploy_scripts_test.sh # The Python in scripts/ was previously untested. A full reseed cannot run - # here -- it needs the WordPress export zip, which is not in the repo, and a - # local Docker stack -- so this covers the parts that can fail silently: the + # here: it needs the WordPress export zip, which is not in the repo, and a + # local Docker stack. This covers the parts that can fail silently: the # scripts compiling at all, the seed generator's id=0 guarantee, and the # reseed script's refusal to destroy data unattended. scripts: @@ -142,7 +142,7 @@ jobs: # The non-TTY guard is the only thing standing between an automated # context and a destroyed database, so assert it stays a guard: this must # FAIL to proceed. Without a Docker daemon the run would abort in - # preflight anyway, so accept either refusal -- what is not acceptable is + # preflight anyway, so accept either refusal. What is not acceptable is # a clean exit, which would mean it sailed past the confirmation. - name: Refuses to destroy data unattended run: | @@ -171,7 +171,7 @@ jobs: fi # A mention of the mode in a comment must not be mistaken for setting - # it -- that regression would silently skip the preamble. + # it; that regression would silently skip the preamble. printf -- "-- NO_AUTO_VALUE_ON_ZERO handled upstream\nINSERT INTO articles (\`author_ids\`, \`comment_status\`) VALUES ('y','open');\n" > "$etl/articles.sql" python ./scripts/generate_wordpress_sql.py "$etl" "$out" if ! head -1 "$out/02-articles.sql" | grep -q '^SET sql_mode'; then diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index dee3df2..557be08 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -109,7 +109,7 @@ jobs: run: echo "value=$(git rev-parse HEAD:embeddings)" >> "$GITHUB_OUTPUT" # This image bakes in ~130MB of model weights, so building it is by far the - # slowest step here -- and it was rebuilt on every push to main, including + # slowest step here, and it was rebuilt on every push to main, including # the overwhelming majority that touch nothing but Go or TypeScript. A tag # that already exists is by definition built from identical content. - name: Check whether the embeddings image already exists diff --git a/.gitignore b/.gitignore index a8a1e69..cd634df 100644 --- a/.gitignore +++ b/.gitignore @@ -19,5 +19,5 @@ deploy/*.env ..env.un~ # Local DB dumps (contain cms_users password hashes and cms_sessions). -# Kept here so Borg backs them up -- /var/lib/docker volumes are NOT backed up. +# Kept here so Borg backs them up; /var/lib/docker volumes are NOT backed up. db-backups/ diff --git a/deploy/cms.env.example b/deploy/cms.env.example index b4af0ac..3bb6d5b 100644 --- a/deploy/cms.env.example +++ b/deploy/cms.env.example @@ -15,7 +15,7 @@ CMS_SESSION_TTL_SECONDS= CMS_REBUILD_TAXONOMY_COUNTS_ON_STARTUP= # Slack classified moderation. The webhook posts the notification; the signing # secret authorizes the Approve/Reject clicks that come back. Setting only one -# of the two leaves the loop broken -- the backend warns at startup which half +# of the two leaves the loop broken; the backend warns at startup which half # is missing. SLACK_CLASSIFIEDS_QUEUE_URL is optional and only adds a link back # to the CMS queue. SLACK_WEBHOOK_URL= @@ -37,12 +37,12 @@ MEDIA_MAX_UPLOAD_BYTES= # embedding reconciler exits at startup. # # EMBED_MODEL must match the WordPress ETL's --embedding-model, and changing it -# is a schema change if the new model is not 384-dimensional -- see +# is a schema change if the new model is not 384-dimensional; see # server/internal/database/schema/article_embeddings.sql. EMBEDDINGS_URL=http://embeddings:8000 EMBED_MODEL=BAAI/bge-small-en-v1.5 # Cores the sidecar may use, out of the host's 6. Uncapped, embedding a freshly # reseeded corpus pins nearly the whole machine for over an hour and starves the -# backends. Raising this speeds up that backfill and nothing else -- ordinary +# backends. Raising this speeds up that backfill and nothing else: ordinary # search only embeds one short query at a time, and caches the result. EMBED_CPUS=4 diff --git a/deploy/compose.cms.yml b/deploy/compose.cms.yml index 048cbee..1c03dd1 100644 --- a/deploy/compose.cms.yml +++ b/deploy/compose.cms.yml @@ -47,7 +47,7 @@ x-backend-base: &backend-base # (91m) and below Cloudflare's 100 MB tunnel limit; raise all of them # together or the smallest one silently wins. MEDIA_MAX_UPLOAD_BYTES: ${MEDIA_MAX_UPLOAD_BYTES:-94371840} - # Semantic search. Shared by both slots -- the sidecar is stateless, so there + # Semantic search. Shared by both slots: the sidecar is stateless, so there # is no reason to run one per slot. Leave empty to run lexical-only search: # the backend then skips query embedding and its reconciler exits at start. EMBEDDINGS_URL: ${EMBEDDINGS_URL-http://embeddings:8000} @@ -80,7 +80,7 @@ x-frontend-base: &frontend-base services: # One shared instance, not one per slot: it holds no state, so both slots can # use it and a deploy never has to wait for a model to load. Not exposed to the - # host -- only the backends talk to it, over the internal network. + # host; only the backends talk to it, over the internal network. embeddings: # Tagged by content, not by commit: CMS_EMBEDDINGS_TAG is the git tree hash # of embeddings/, so it only changes when the sidecar itself changes. Tagging @@ -98,8 +98,8 @@ services: # without OMP_NUM_THREADS, onnxruntime still spawns one thread per visible # core and thrashes against that ceiling instead of respecting it. # - # This trades backfill throughput for headroom -- roughly 2 articles/sec - # becomes 1.4 -- which is the right way round: the backfill is background + # This trades backfill throughput for headroom (roughly 2 articles/sec + # becomes 1.4) which is the right way round: the backfill is background # work that converges on its own, and nothing waits on it. cpus: ${EMBED_CPUS:-4} environment: diff --git a/deploy/scripts/common.sh b/deploy/scripts/common.sh index 2f31b9c..13eb5f9 100755 --- a/deploy/scripts/common.sh +++ b/deploy/scripts/common.sh @@ -246,7 +246,7 @@ wait_for_url() { } # wait_for_embeddings polls the container's health state rather than an HTTP -# endpoint, because the sidecar is deliberately not published to the host -- only +# endpoint, because the sidecar is deliberately not published to the host: only # the backends reach it, over the compose network. Its healthcheck 503s until the # model has finished loading, so "healthy" here means it can actually answer. wait_for_embeddings() { diff --git a/deploy/scripts/deploy.sh b/deploy/scripts/deploy.sh index 53a3f88..d6c8009 100755 --- a/deploy/scripts/deploy.sh +++ b/deploy/scripts/deploy.sh @@ -22,7 +22,7 @@ export CMS_IMAGE_TAG # The sidecar image is tagged by content: the git tree hash of embeddings/, # which changes only when something in that directory does. Publish skips the # build when that tag already exists, and Compose leaves the running container -# alone when the tag is unchanged -- so a Go-only commit no longer rebuilds a +# alone when the tag is unchanged, so a Go-only commit no longer rebuilds a # ~130MB model image in CI, nor reloads the model on the host for ~60s. # # Derived from the trusted default-branch checkout rather than from @@ -55,7 +55,7 @@ echo "deploying ${CMS_IMAGE_TAG} to inactive slot: ${next_slot}" # The embedding sidecar is shared by both slots rather than duplicated per slot: # it is stateless, so a second copy would only cost memory on a host that has # little to spare. That means it is not part of the blue/green swap and has to be -# brought up separately -- the slot services below are started with --no-deps. +# brought up separately; the slot services below are started with --no-deps. # # Deliberately never fatal. Search degrades to lexical-only when the sidecar is # missing or still loading its model, so a sidecar problem must not block or roll diff --git a/deploy/scripts/deploy_scripts_test.sh b/deploy/scripts/deploy_scripts_test.sh index 83d57df..0699c53 100755 --- a/deploy/scripts/deploy_scripts_test.sh +++ b/deploy/scripts/deploy_scripts_test.sh @@ -56,7 +56,7 @@ make_case() { PUBLIC_HEALTH_TIMEOUT=0 # The sidecar wait polls container health, which never turns healthy against # the fake compose in these tests. Left at its 240s production default it made - # every deploy case sit out the full timeout -- the suite took 8 minutes. + # every deploy case sit out the full timeout; the suite took 8 minutes. EMBEDDINGS_HEALTH_TIMEOUT=0 DEPLOY_TEST_MODE=1 NGINX_TEST_CMD='exit "${FAKE_NGINX_TEST_STATUS:-0}"' diff --git a/docker-compose.yml b/docker-compose.yml index 8e0bfd0..ac442de 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -35,7 +35,7 @@ services: restart: unless-stopped # Capped for the same reason as production (see deploy/compose.cms.yml): # uncapped, embedding a freshly seeded corpus saturates every core on the - # machine. Both settings are needed and should match -- cpus is the ceiling, + # machine. Both settings are needed and should match: cpus is the ceiling, # OMP_NUM_THREADS stops onnxruntime spawning a thread per core regardless. cpus: ${EMBED_CPUS:-4} environment: @@ -50,7 +50,7 @@ services: retries: 5 start_period: 60s # Stateless by design: no volume, nothing to back up. Not published to the - # host either -- only the CMS talks to it, over the internal network. + # host either; only the CMS talks to it, over the internal network. networks: - triangle_net diff --git a/embeddings/app.py b/embeddings/app.py index a7c568e..9a8eeb6 100644 --- a/embeddings/app.py +++ b/embeddings/app.py @@ -6,8 +6,8 @@ meaningless, so both go through this one service and it reports which model produced them. -It is deliberately stateless. Nothing here is a source of truth -- the vectors -live in MariaDB -- so it needs no volume, no backup, and no reconciliation. If +It is deliberately stateless. Nothing here is a source of truth (the vectors +live in MariaDB) so it needs no volume, no backup, and no reconciliation. If it restarts, or is missing entirely, the CMS degrades to lexical search. """ @@ -27,7 +27,7 @@ # BGE is an asymmetric retrieval model: it was trained with short queries # prefixed and documents bare. Embedding a query without this prefix quietly -# costs a chunk of retrieval quality -- it still returns vectors, just worse +# costs a chunk of retrieval quality: it still returns vectors, just worse # ones, which is the kind of bug that never surfaces as an error. QUERY_PREFIX = os.getenv( "EMBED_QUERY_PREFIX", @@ -96,7 +96,7 @@ def embed(request: EmbedRequest) -> EmbedResponse: # Normalize explicitly rather than trusting the model wrapper's default. # MariaDB ranks these with euclidean distance, which only agrees with cosine - # similarity on unit vectors -- the previous ETL skipped this, so magnitude + # similarity on unit vectors. The previous ETL skipped this, so magnitude # leaked into every "related articles" ranking. norms = np.linalg.norm(vectors, axis=1, keepdims=True) vectors = vectors / np.clip(norms, 1e-12, None) diff --git a/frontend/nginx.conf b/frontend/nginx.conf index 076409e..4092fe1 100644 --- a/frontend/nginx.conf +++ b/frontend/nginx.conf @@ -40,7 +40,7 @@ server { } # index.html carries the references to those hashed filenames, so it is the - # one file that must never be cached -- a stale copy points the browser at + # one file that must never be cached: a stale copy points the browser at # assets the last deploy removed. location = /index.html { add_header Cache-Control "no-cache"; diff --git a/frontend/src/auth/adminOnlyNoticeContext.ts b/frontend/src/auth/adminOnlyNoticeContext.ts index 293b600..94cdfa5 100644 --- a/frontend/src/auth/adminOnlyNoticeContext.ts +++ b/frontend/src/auth/adminOnlyNoticeContext.ts @@ -11,7 +11,7 @@ export const AdminOnlyNoticeContext = createContext /** * Access to the shared "admin only" dialog. * - * Unlike useSessionAuth this does not throw when no provider is mounted -- it + * Unlike useSessionAuth this does not throw when no provider is mounted; it * degrades to a no-op. useApiFetch calls it on every request, and a missing * provider should not take down a page (or a test) that never triggers a 403. */ diff --git a/frontend/src/auth/urls.ts b/frontend/src/auth/urls.ts index 7bb25ce..4a74643 100644 --- a/frontend/src/auth/urls.ts +++ b/frontend/src/auth/urls.ts @@ -21,7 +21,7 @@ export function publicSiteUrl() { // The public permalink for a slug. The URL is fully determined by the slug, so // it can be handed out (newsletter, social scheduling) before the article is -// published -- it 404s until then, and resolves the moment it goes live. +// published. It 404s until then, and resolves the moment it goes live. export function articleUrl(slug: string) { return `${publicSiteUrl()}/article/${encodeURIComponent(slug)}` } diff --git a/frontend/src/components/MediaPicker.tsx b/frontend/src/components/MediaPicker.tsx index d3e4608..fdb4c73 100644 --- a/frontend/src/components/MediaPicker.tsx +++ b/frontend/src/components/MediaPicker.tsx @@ -23,7 +23,7 @@ type MediaPickerProps = { // When set, the picker also accepts a bare image URL. Only the featured-image // field wants this: an article's photo_url is served verbatim, so it can point // at an image that was never in our library. Body attachments have no such - // escape hatch by design -- they get sideloaded so articles never hotlink. + // escape hatch by design: they get sideloaded so articles never hotlink. onUseUrl?: (url: string) => void initialUrl?: string } @@ -147,7 +147,7 @@ function MediaPicker({ onSelect, onClose, title = "Insert image", onUseUrl, init }, [sentinel, hasMore, isLoading, isLoadingMore, items.length]) // Alt text saved from a tile belongs to the library record, so the grid has to - // show the new value straight away -- otherwise the "No alt text" warning + // show the new value straight away; otherwise the "No alt text" warning // stays up on an image that now has some. const updateItem = useCallback((id: number, altText: string) => { setItems((prev) => prev.map((item) => (item.id === id ? { ...item, alt_text: altText } : item))) @@ -344,7 +344,7 @@ type AltTextFieldProps = { * Inline alt-text editor for one tile. * * It writes to the library record (PATCH /v1/media/{id}), not to this article, - * so an image described once is described everywhere it is used -- the same + * so an image described once is described everywhere it is used, the same * contract the picker already relies on when it hands alt_text to the editor. * * Its own error state rather than the picker's banner: a failed save belongs diff --git a/frontend/src/components/TrixEditor.css b/frontend/src/components/TrixEditor.css index 48f480a..d934f28 100644 --- a/frontend/src/components/TrixEditor.css +++ b/frontend/src/components/TrixEditor.css @@ -285,7 +285,7 @@ trix-editor.trix-content .attachment__caption .attachment__size { /* An image with no caption yet. Trix marks the empty
with data-trix-placeholder ("Add a caption…") but ships no CSS for it, so the - element collapses to zero height -- invisible, and with nothing to click. + element collapses to zero height: invisible, and with nothing to click. Rendering the placeholder gives the author both the prompt and the target. */ trix-editor.trix-content .attachment__caption[data-trix-placeholder]:empty::before { content: attr(data-trix-placeholder); @@ -447,8 +447,8 @@ trix-editor.trix-content .attachment__toolbar .attachment__metadata-container { /* Remove. Trix draws this as a white circle with a 2px `highlight` (system accent) border and hides the text label behind text-indent, showing an ✕ via - a ::before background image. Keep the icon -- undoing the text-indent would - leave the button empty -- and drop only the standalone-circle chrome, which + a ::before background image. Keep the icon (undoing the text-indent would + leave the button empty) and drop only the standalone-circle chrome, which looks wrong now that it sits inside a grouped toolbar. */ trix-editor.trix-content .attachment__toolbar .trix-button--remove { position: relative; @@ -476,8 +476,8 @@ trix-editor.trix-content .attachment.attachment--preview.attachment--dragging { cursor: grabbing; } -/* Per-image alignment. No longer settable from the editor -- the public site - sizes article images at width:100%, which leaves nothing for a float to do -- +/* Per-image alignment. No longer settable from the editor, since the public + site sizes article images at width:100%, which leaves nothing for a float to do, but alignment already present on migrated WordPress content is preserved through a save, so it is still reflected here to match what will publish. @@ -486,7 +486,7 @@ trix-editor.trix-content .attachment.attachment--preview.attachment--dragging { as far as Trix's MutationObserver is concerned: it re-parses the document, re-renders it and restores its own selection, which yanks the caret (and the focus) back into the article from wherever the author had moved it. Doing - that on every trix-change -- i.e. on every keystroke -- made the editor + that on every trix-change, i.e. on every keystroke, made the editor un-leavable while an image was selected. */ trix-editor.trix-content .attachment--preview[data-trix-attributes*='"align":"left"'] { text-align: left; diff --git a/frontend/src/components/TrixEditor.tsx b/frontend/src/components/TrixEditor.tsx index 80ed961..4c3de6c 100644 --- a/frontend/src/components/TrixEditor.tsx +++ b/frontend/src/components/TrixEditor.tsx @@ -13,8 +13,8 @@ import { // Leave the caption area of an image empty until it has a real caption. Trix's // default is to fill it with the filename and file size, which reads as a -// caption the author didn't write, and -- because the slot is then never empty -// -- suppresses the "Add a caption…" placeholder that tells them it is editable. +// caption the author didn't write, and (because the slot is then never empty) +// suppresses the "Add a caption…" placeholder that tells them it is editable. // Non-previewable file attachments are unaffected: Trix forces the name on for // those, and a file stub with no label would be nothing at all. if (typeof window !== "undefined" && window.Trix) { @@ -35,8 +35,8 @@ if (typeof window !== "undefined" && window.Trix) { // ["caption", "presentation"] and removeProhibitedAttributes drops everything // else as soon as the piece is built, so `align` only ever lived on the live // figure as a class our own effect re-applied. That class was enough to make - // alignment look preserved until Trix re-rendered the figure from the piece - // -- which editing a caption does -- at which point the alignment silently + // alignment look preserved until Trix re-rendered the figure from the piece, + // which editing a caption does, at which point the alignment silently // vanished from both the editor and the saved article. if (!window.Trix.AttachmentPiece.permittedAttributes.includes("align")) { window.Trix.AttachmentPiece.permittedAttributes.push("align"); @@ -97,7 +97,7 @@ function TrixEditor({ value, onChange }: TrixEditorProps) { const editorRef = useRef(null); const wrapperRef = useRef(null); // Every HTML string we have emitted since the last load from outside, so we - // never call loadHTML on our own output -- which would reset the document and + // never call loadHTML on our own output, which would reset the document and // drop the caret at the top of the article mid-edit. // // This has to be a set of everything emitted, not just the most recent one. @@ -173,7 +173,7 @@ function TrixEditor({ value, onChange }: TrixEditorProps) { // Trix decides an attachment is no longer being edited by watching the // document's selection: on the first selectionchange whose range is not the // attachment's own, it stops editing, tears the caption field down and - // re-renders -- and that render puts the selection, and with it the browser's + // re-renders, and that render puts the selection, and with it the browser's // focus, back inside the article. While an image sat in that state the check // was still armed after the author had clicked away, and Firefox raises // selectionchange on the document for typing in a plain (Chrome @@ -185,13 +185,13 @@ function TrixEditor({ value, onChange }: TrixEditorProps) { // asking Trix to stop editing the attachment runs the same teardown and steals // the focus just as thoroughly. A selection left inside a contenteditable // nobody is focused on has nothing to draw and nothing to move, so clearing it - // costs the author nothing -- Trix remembers the caret separately and puts it - // back when the editor is focused again -- and it leaves Trix reading no range + // costs the author nothing (Trix remembers the caret separately and puts it + // back when the editor is focused again) and it leaves Trix reading no range // at all rather than a stale one, which is what makes the check fall through. // // focusout rather than trix-blur: while a caption is open the focus is on the // caption field, not on the editor element, so the editor never blurs and - // trix-blur never comes -- and that is exactly the state this is here for. + // trix-blur never comes, and that is exactly the state this is here for. // Where the focus went is read a tick later, since focusout fires before it // has landed. Measured against the wrapper, not the editor, so that stepping // into our own chrome (the alt-text dialog, the media picker) is not treated @@ -228,7 +228,7 @@ function TrixEditor({ value, onChange }: TrixEditorProps) { // mutation inside Trix's contenteditable: its MutationObserver re-parsed the // document, re-rendered it, and restored its own selection with it. Doing // that on every trix-change meant every keystroke in an image caption pulled - // the caret -- and the browser's focus -- back into the article, so clicking + // the caret, and the browser's focus, back into the article, so clicking // out to the author or section search box and typing put the first letter in // the search box and the rest back in the story. @@ -288,7 +288,7 @@ function TrixEditor({ value, onChange }: TrixEditorProps) { const { url } = JSON.parse(xhr.responseText) as { url: string }; // href only for non-images. Trix wraps an attachment carrying an href // in an , which for a previewable image swallows every click on the - // figure -- including the caption field. A file stub, by contrast, has + // figure, including the caption field. A file stub, by contrast, has // nothing to edit and a download link is the whole point of it. attachment.setAttributes( file.type.startsWith("image/") ? { url } : { url, href: url }, @@ -421,7 +421,7 @@ function TrixEditor({ value, onChange }: TrixEditorProps) { } openBlockForImage(); - // Deliberately no href -- see uploadFile. The saved article is a plain + // Deliberately no href; see uploadFile. The saved article is a plain //
either way, so the link would only ever have existed inside // the editor, where it fights with selecting and captioning the image. const attachment = new Trix.Attachment({ @@ -474,7 +474,7 @@ function TrixEditor({ value, onChange }: TrixEditorProps) { // Article-scoped by design: the attachment carries no library id (nothing // survives a reload but the URL), and an image can legitimately want a // different description in a different story. Setting the library's own alt - // text -- the value every future insertion starts from -- is the picker's job. + // text, the value every future insertion starts from, is the picker's job. const saveAltText = useCallback((attachmentId: number, alt: string) => { setAltEditor(null); const editor = editorRef.current; @@ -505,7 +505,7 @@ function TrixEditor({ value, onChange }: TrixEditorProps) { // which overrides anything an author sets, so both were controls that appeared // to work in the editor and changed nothing on the published page. Reordering // is kept because it does survive. Alignment already stored on legacy - // WordPress content is still preserved through a save -- it just can no longer + // WordPress content is still preserved through a save; it just can no longer // be set from here. useEffect(() => { const editor = editorRef.current; @@ -572,13 +572,13 @@ function TrixEditor({ value, onChange }: TrixEditorProps) { return null; }; - // Where the dragged block would land, as a boundary index -- or null if it + // Where the dragged block would land, as a boundary index, or null if it // cannot go anywhere from here. // // Only boundaries clear of the dragged block itself are candidates: the two // touching it leave the document exactly as it was. They used to be offered, // and since an image block is as tall as the image, the pointer spent the - // whole of a short drag inside them -- the indicator appeared, tracked the + // whole of a short drag inside them: the indicator appeared, tracked the // pointer, and then the drop did nothing. Moving an image at all meant // dragging clear past its own block and half of the next one, with the // indicator claiming otherwise the entire way. That is the bug this shape @@ -638,8 +638,8 @@ function TrixEditor({ value, onChange }: TrixEditorProps) { }; // Re-resolve a figure by id. The element captured when a gesture started may - // already be detached: selecting an attachment -- which Trix does on the very - // mousedown that starts a drag -- re-renders it, so an identical figure has + // already be detached: selecting an attachment, which Trix does on the very + // mousedown that starts a drag, re-renders it, so an identical figure has // taken its place. Anything that asks "which block is this image in?" has to // go through here or it gets the answer for a corpse. const liveFigure = (figure: HTMLElement): HTMLElement | null => { @@ -659,7 +659,7 @@ function TrixEditor({ value, onChange }: TrixEditorProps) { : -1; }; - // Put an attachment into Trix's "being edited" state -- the state that + // Put an attachment into Trix's "being edited" state, the one that // shows its caption field and its toolbar. Trix only enters it from a // mousedown of its own, which is no help either when that mousedown's // selection gets reset (see onUp) or after a move has replaced the element. @@ -679,8 +679,8 @@ function TrixEditor({ value, onChange }: TrixEditorProps) { // Move the figure's whole block to sit before or after the target block. // // This reorders Trix's serialized HTML and reloads it, rather than splicing - // the live document through Trix's mutation APIs. The obvious approach -- - // remove the attachment, then insert it at a captured DOM Range -- cannot + // the live document through Trix's mutation APIs. The obvious approach, + // removing the attachment then inserting it at a captured DOM Range, cannot // work: the Attachment objects on editor.getDocument() are plain models // with no remove() of their own (only the ManagedAttachment passed to // trix-attachment-add has one), and any removal re-renders the blocks the @@ -721,7 +721,7 @@ function TrixEditor({ value, onChange }: TrixEditorProps) { // Swap the document in under a recorded undo entry, rather than through // loadHTML. loadHTML routes to Editor#loadSnapshot, which replaces the - // whole UndoManager -- so a move was not merely un-undoable, it silently + // whole UndoManager, so a move was not merely un-undoable: it silently // threw away every undo step the author had built up before it. // recordUndoEntry snapshots the current document and selection onto the // stack, and Composition#setDocument then mutates without disturbing it. @@ -738,8 +738,8 @@ function TrixEditor({ value, onChange }: TrixEditorProps) { editor.editor.composition.setDocument(movedDocument); // Re-select the image at its new home. Without this every nudge costs the - // author the selection -- and with it the toolbar they are clicking -- - // so moving an image three blocks would mean three round trips to it. + // author the selection, and with it the toolbar they are clicking, so + // moving an image three blocks would mean three round trips to it. // // Two frames, not one. One frame is enough for the element to exist, but // not for Trix to have finished rendering the reloaded document, and @@ -840,12 +840,12 @@ function TrixEditor({ value, onChange }: TrixEditorProps) { // This is the whole reason the gesture is on pointer events: with plain // mouse events, anything that starts a native drag session mid-gesture // (see the dragstart guard below) makes the browser stop delivering - // mousemove and -- fatally -- mouseup, so the release was never seen and + // mousemove and, fatally, mouseup, so the release was never seen and // nothing was ever committed. Capture is retargeted to the wrapper // rather than the figure because Trix re-renders the figure during a // drag; capturing on an element that then leaves the document drops the // capture with it. Capture is taken here and not on pointerdown so that - // a plain click still reaches Trix untouched -- capturing at pointerdown + // a plain click still reaches Trix untouched: capturing at pointerdown // retargets the compatibility mousedown too, which is the event Trix // selects the attachment from. try { @@ -859,7 +859,7 @@ function TrixEditor({ value, onChange }: TrixEditorProps) { liveFigure(figure)?.classList.add("attachment--dragging"); document.body.style.cursor = "grabbing"; // Drop whatever text selection the un-prevented pointerdown started, so - // the drag doesn't paint a selection highlight across the article -- and + // the drag doesn't paint a selection highlight across the article, and // so there is no selection left for the browser to want to drag. window.getSelection()?.removeAllRanges(); scroller = scrollContainer(); @@ -904,7 +904,7 @@ function TrixEditor({ value, onChange }: TrixEditorProps) { dropIndicator.style.display = "none"; }; - // The pointer was taken away mid-gesture -- a touch turning into a scroll, + // The pointer was taken away mid-gesture: a touch turning into a scroll, // the window losing the device. Abandon the move rather than committing to // wherever the indicator happened to be. const onCancel = (cancelEvent: PointerEvent) => { @@ -917,7 +917,7 @@ function TrixEditor({ value, onChange }: TrixEditorProps) { if (upEvent.pointerId !== pointerId) return; activeGestureCleanup = null; // Read before cleanup, which clears `dragging` to stop the auto-scroll - // loop -- reading after it would make every drop look like a click. + // loop. Reading after it would make every drop look like a click. const wasDragging = dragging; const releasedOn = dropTarget; cleanup(); @@ -928,7 +928,7 @@ function TrixEditor({ value, onChange }: TrixEditorProps) { // Re-assert Trix's own attachment selection. Trix makes it on // mousedown, but when the click is what focuses the editor in the // first place, the focus that follows resets the selection and takes - // the attachment toolbar back down with it -- so the first click on + // the attachment toolbar back down with it, so the first click on // an image in a freshly loaded editor appeared to do nothing, and it // took a second click to get at Remove or the move buttons. Trix // ignores this when the attachment is already the one being edited. @@ -951,9 +951,9 @@ function TrixEditor({ value, onChange }: TrixEditorProps) { //