From f81692943ea2d58b1da5923f1351ebad409d914e Mon Sep 17 00:00:00 2001 From: Aprillion Date: Thu, 6 Jul 2023 10:01:20 +0200 Subject: [PATCH 1/6] support multiple footnote references * and don't crash when a footnote reference does not exist --- app/routes/questions/$question.tsx | 11 +++++-- app/server-utils/parsing-utils.spec.ts | 41 +++++++++++++++++++++++++- app/server-utils/parsing-utils.ts | 4 +-- 3 files changed, 50 insertions(+), 6 deletions(-) diff --git a/app/routes/questions/$question.tsx b/app/routes/questions/$question.tsx index 0517e614..428bed8e 100644 --- a/app/routes/questions/$question.tsx +++ b/app/routes/questions/$question.tsx @@ -183,9 +183,11 @@ const updateTextNodes = (el: Node, textProcessor: (node: Node) => Node) => { function Contents({pageid, html, glossary}: {pageid: PageId; html: string; glossary: Glossary}) { const elementRef = useRef(null) - const footnoteHTML = (el: HTMLDivElement, e: HTMLAnchorElement): string => { + const footnoteHTML = (el: HTMLDivElement, e: HTMLAnchorElement): string | null => { const id = e.getAttribute('href') || '' - const footnote = el.querySelector(id) as HTMLLabelElement + const footnote = el.querySelector(id) + + if (!footnote) return null const elem = document.createElement('div') elem.innerHTML = footnote.innerHTML @@ -283,7 +285,10 @@ function Contents({pageid, html, glossary}: {pageid: PageId; html: string; gloss // In theory this could be extended to all links el.querySelectorAll('.footnote-ref > a').forEach((e) => - addPopup(e as HTMLAnchorElement, footnoteHTML(el, e as HTMLAnchorElement)) + { + const footnote = footnoteHTML(el, e as HTMLAnchorElement) + if (footnote) addPopup(e as HTMLAnchorElement, footnote) + } ) }, [html, glossary, pageid]) diff --git a/app/server-utils/parsing-utils.spec.ts b/app/server-utils/parsing-utils.spec.ts index a49d252a..7771987e 100644 --- a/app/server-utils/parsing-utils.spec.ts +++ b/app/server-utils/parsing-utils.spec.ts @@ -1,4 +1,4 @@ -import {urlToIframe, externalLinksOnNewTab} from './parsing-utils' +import {urlToIframe, uniqueFootnotes, externalLinksOnNewTab} from './parsing-utils' describe('urlToIframe', () => { describe('should not convert a markdown link', () => { @@ -38,6 +38,45 @@ describe('urlToIframe', () => { }) }) +describe('uniqueFootnotes', () => { + test('no link', () => { + expect(uniqueFootnotes('gg', '7757')).toBe('gg') + }) + + test('multiple footnotes, one of them repeated', () => { + expect( + uniqueFootnotes( + `x[1] + y[1:1] + z[2] + --- +
    +
  1. + gg x y +
    2. +
  2. + gg x +
    3. +
`, + '7757' + ) + ).toBe( + `x[1] + y[1:1] + z[2] + --- +
    +
  1. + gg x y +
    2. +
  2. + gg x +
    3. +
` + ) + }) +}) + describe('externalLinksOnNewTab', () => { test('no link', () => { expect(externalLinksOnNewTab('gg')).toBe('gg') diff --git a/app/server-utils/parsing-utils.ts b/app/server-utils/parsing-utils.ts index 0c193089..949c8481 100644 --- a/app/server-utils/parsing-utils.ts +++ b/app/server-utils/parsing-utils.ts @@ -67,11 +67,11 @@ export const uniqueFootnotes = (html: string, pageid: string): string => { // Make sure the footnotes point to unique ids. This is very ugly and would be better handled // with a proper parser, but should do the trick so is fine? Maybe? html = html.replace( - //g, + //g, `` ) html = html.replace( - //g, + //g, `` ) html = html.replace( From 576b310c3642daa7e7509b9a32aa5aa9ff571b34 Mon Sep 17 00:00:00 2001 From: Aprillion Date: Thu, 6 Jul 2023 10:01:26 +0200 Subject: [PATCH 2/6] enable sourcemap for production build * for better error messages next time * our server code is open source anyway, so not a security issue --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index d1989b96..d33cab8d 100644 --- a/package.json +++ b/package.json @@ -53,7 +53,7 @@ "eslint:fix": "eslint --fix --ignore-pattern .gitignore \"**/*.ts*\"", "prettier:fix": "prettier --write --ignore-path .gitignore \"**/*.{ts*,js,css,md}\"", "lint:fix": "tsc && npm run prettier:fix && npm run eslint:fix", - "build": "rimraf public/build && npm run generate-icons && cross-env NODE_ENV=production remix build", + "build": "rimraf public/build && npm run generate-icons && cross-env NODE_ENV=production remix build --sourcemap", "dev:remix": "cross-env NODE_ENV=development remix watch", "dev:miniflare": "cross-env NODE_ENV=development miniflare ./build/index.js --watch", "start": "npm run generate-icons && cross-env NODE_ENV=development remix build && run-p dev:*", From b0be0a20e2152a36330eace9bc6f689e47a46074 Mon Sep 17 00:00:00 2001 From: Aprillion Date: Thu, 6 Jul 2023 10:03:30 +0200 Subject: [PATCH 3/6] rerun prettier --- README.md | 15 ++++++++------- app/root.css | 2 +- app/routes/questions/$question.tsx | 10 ++++------ 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 84e8a90e..9ed197bc 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,7 @@ Contributions are welcome, the code is released under the MIT License. If you'd - [Install Node.js and npm](https://docs.npmjs.com/downloading-and-installing-node-js-and-npm) - [Create Cloudflare account](https://dash.cloudflare.com/) - [Install Wrangler CLI](https://developers.cloudflare.com/workers/wrangler/install-and-update/#install-wrangler-locally) - and [authenticate the CLI](https://developers.cloudflare.com/workers/wrangler/install-and-update/#install-wrangler-locally) + and [authenticate the CLI](https://developers.cloudflare.com/workers/wrangler/install-and-update/#install-wrangler-locally) 2. Clone the [Repo](https://github.com/StampyAI/stampy-ui) @@ -31,23 +31,24 @@ Contributions are welcome, the code is released under the MIT License. If you'd 4. Setup in [Coda.io](https://coda.io/account) - **4.1 (Required)** Setup read access to the API view in Coda - > *Note*: + + > _Note_: > Content in Coda comes from parsing Google Docs (so that site visitors can make suggestions that > will be reviewed by our editors). The parser lives in the > [GDocsRelatedThings](https://github.com/StampyAI/GDocsRelatedThings/#readme) repo. - When logged in to Coda, `Generate API token` in your Account settigns - Add restrictions: `Doc or table`, `Read only`, for the doc with url `https://coda.io/d/_dfau7sl2hmG` - (you need access to the doc of course, which you can request on the Discord) + (you need access to the doc of course, which you can request on the Discord) - Replace the value for `{CODA_TOKEN}` in `wrangler.toml` - **4.2 (Optional)** Setup write access to the API write view in Coda - > Note: This step is only needed for incrementing counters (helpful, etc.). There isn't a test environment, so any changes there will also effect the live site, so think twice before using them. + > Note: This step is only needed for incrementing counters (helpful, etc.). There isn't a test environment, so any changes there will also effect the live site, so think twice before using them. - - When logged in to Coda, `Generate API token` in your Account settings - - Add restrictions: `Doc or table`, `Read and Write`, for the table with url `https://coda.io/d/_dfau7sl2hmG#_tutable-eEhx2YPsBE` - - Replace the value for `{CODA_WRITES_TOKEN}` in `wrangler.toml` + - When logged in to Coda, `Generate API token` in your Account settings + - Add restrictions: `Doc or table`, `Read and Write`, for the table with url `https://coda.io/d/_dfau7sl2hmG#_tutable-eEhx2YPsBE` + - Replace the value for `{CODA_WRITES_TOKEN}` in `wrangler.toml` - **4.3 (Optional)** Setup write access to the "Incoming questions" table in Coda diff --git a/app/root.css b/app/root.css index 4831d79d..f2cb0ebe 100644 --- a/app/root.css +++ b/app/root.css @@ -660,7 +660,7 @@ a.see-more.visible:after { content: 'See less'; } -a[target="_blank"]:not(.icon-link):after { +a[target='_blank']:not(.icon-link):after { content: ' '; display: inline-block; vertical-align: top; diff --git a/app/routes/questions/$question.tsx b/app/routes/questions/$question.tsx index 428bed8e..be79f840 100644 --- a/app/routes/questions/$question.tsx +++ b/app/routes/questions/$question.tsx @@ -284,12 +284,10 @@ function Contents({pageid, html, glossary}: {pageid: PageId; html: string; gloss updateTextNodes(el, insertGlossary) // In theory this could be extended to all links - el.querySelectorAll('.footnote-ref > a').forEach((e) => - { - const footnote = footnoteHTML(el, e as HTMLAnchorElement) - if (footnote) addPopup(e as HTMLAnchorElement, footnote) - } - ) + el.querySelectorAll('.footnote-ref > a').forEach((e) => { + const footnote = footnoteHTML(el, e as HTMLAnchorElement) + if (footnote) addPopup(e as HTMLAnchorElement, footnote) + }) }, [html, glossary, pageid]) return ( From aa81224c8fccea0e49f7f7e14d94a89353b60957 Mon Sep 17 00:00:00 2001 From: Aprillion Date: Thu, 6 Jul 2023 11:28:55 +0200 Subject: [PATCH 4/6] show related questions on lazy load from search results #247 --- app/hooks/useQuestionStateInUrl.ts | 6 ++++-- app/routes/questions/$question.tsx | 28 +++++++++++----------------- 2 files changed, 15 insertions(+), 19 deletions(-) diff --git a/app/hooks/useQuestionStateInUrl.ts b/app/hooks/useQuestionStateInUrl.ts index d0056f12..2579d7ef 100644 --- a/app/hooks/useQuestionStateInUrl.ts +++ b/app/hooks/useQuestionStateInUrl.ts @@ -196,7 +196,7 @@ export default function useQuestionStateInUrl(minLogo: boolean, initialQuestions * are on the page */ const toggleQuestion = useCallback( - (questionProps: Question, options?: {moveToTop?: boolean}) => { + (questionProps: Question, options?: {moveToTop?: boolean; onlyRelated?: boolean}) => { const {pageid, relatedQuestions} = questionProps let currentState = stateString ?? initialCollapsedState @@ -212,7 +212,9 @@ export default function useQuestionStateInUrl(minLogo: boolean, initialQuestions } const newRelatedQuestions = unshownRelatedQuestions(questions, questionProps) - const newState = insertIntoState(currentState, pageid, newRelatedQuestions) + const newState = insertIntoState(currentState, pageid, newRelatedQuestions, { + toggle: !options?.onlyRelated, + }) updateStateString(newState) }, diff --git a/app/routes/questions/$question.tsx b/app/routes/questions/$question.tsx index be79f840..1179849f 100644 --- a/app/routes/questions/$question.tsx +++ b/app/routes/questions/$question.tsx @@ -77,7 +77,15 @@ export function Question({ const title = codaStatus && codaStatus !== QuestionStatus.LIVE_ON_SITE ? `WIP - ${codaTitle}` : codaTitle const isLoading = useRef(false) - const refreshOnToggleAfterLoading = useRef(false) + + const isExpanded = questionState === '_' + const isRelated = questionState === 'r' + const clsExpanded = isExpanded ? 'expanded' : isRelated ? 'related' : 'collapsed' + + const [isLinkHovered, setLinkHovered] = useState(false) + const clsLinkHovered = isLinkHovered ? 'link-hovered' : '' + const cls = `${clsExpanded} ${clsLinkHovered}` + useEffect(() => { if (text == null && !isLoading.current) { isLoading.current = true @@ -85,27 +93,13 @@ export function Question({ fetchQuestion(pageid).then((newQuestionProps) => { if (!newQuestionProps) return onLazyLoadQuestion(newQuestionProps) - if (refreshOnToggleAfterLoading.current) { - onToggle(newQuestionProps) - refreshOnToggleAfterLoading.current = false - } + if (isExpanded) onToggle(newQuestionProps, {onlyRelated: true}) }) } - }, [pageid, text, onLazyLoadQuestion, onToggle]) - - const isExpanded = questionState === '_' - const isRelated = questionState === 'r' - const clsExpanded = isExpanded ? 'expanded' : isRelated ? 'related' : 'collapsed' - - const [isLinkHovered, setLinkHovered] = useState(false) - const clsLinkHovered = isLinkHovered ? 'link-hovered' : '' - const cls = `${clsExpanded} ${clsLinkHovered}` + }, [pageid, text, onLazyLoadQuestion, onToggle, isExpanded]) const handleToggle = () => { onToggle(questionProps) - if (text == null) { - refreshOnToggleAfterLoading.current = true - } } let html From 30421ae13dc155ab9374fc6e6ef393c360e66ed3 Mon Sep 17 00:00:00 2001 From: Aprillion Date: Thu, 6 Jul 2023 13:17:24 +0200 Subject: [PATCH 5/6] scroll to top AFTER a question is added to top of state --- app/hooks/useQuestionStateInUrl.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/app/hooks/useQuestionStateInUrl.ts b/app/hooks/useQuestionStateInUrl.ts index 2579d7ef..e6286fa0 100644 --- a/app/hooks/useQuestionStateInUrl.ts +++ b/app/hooks/useQuestionStateInUrl.ts @@ -124,9 +124,12 @@ export default function useQuestionStateInUrl(minLogo: boolean, initialQuestions } const moveToTop = (currentState: string, {pageid}: Question) => { - window.scrollTo({ - top: 0, - behavior: 'smooth', + setTimeout(() => { + // scroll to top after the state is updated + window.scrollTo({ + top: 0, + behavior: 'smooth', + }) }) return moveQuestionToTop(currentState, pageid) } From 8f9f0f61c2a30d97d82375b0930f63c47d40acc3 Mon Sep 17 00:00:00 2001 From: Aprillion Date: Thu, 6 Jul 2023 13:30:52 +0200 Subject: [PATCH 6/6] use QuestionState enum instead of cryptic strings --- app/routes/questions/$question.tsx | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/app/routes/questions/$question.tsx b/app/routes/questions/$question.tsx index 1179849f..5934cfe2 100644 --- a/app/routes/questions/$question.tsx +++ b/app/routes/questions/$question.tsx @@ -1,5 +1,10 @@ import type {LoaderArgs} from '@remix-run/cloudflare' -import {GlossaryEntry, loadQuestionDetail, QuestionStatus} from '~/server-utils/stampy' +import { + GlossaryEntry, + loadQuestionDetail, + QuestionState, + QuestionStatus, +} from '~/server-utils/stampy' import {useRef, useEffect, useState} from 'react' import AutoHeight from 'react-auto-height' import type {Question, Glossary, PageId} from '~/server-utils/stampy' @@ -78,8 +83,8 @@ export function Question({ codaStatus && codaStatus !== QuestionStatus.LIVE_ON_SITE ? `WIP - ${codaTitle}` : codaTitle const isLoading = useRef(false) - const isExpanded = questionState === '_' - const isRelated = questionState === 'r' + const isExpanded = questionState === QuestionState.OPEN + const isRelated = questionState === QuestionState.RELATED const clsExpanded = isExpanded ? 'expanded' : isRelated ? 'related' : 'collapsed' const [isLinkHovered, setLinkHovered] = useState(false)