From e034a2824260e209c2366e416ff1d511c95bf4c0 Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Sat, 25 Apr 2026 08:25:55 -0400 Subject: [PATCH] chore: Address AI code quality findings (#12163) - Modal: translate default title and bind Dialog.Title to visible text - Document Header: regroup imports and rename isNew -> wasNew - Redis adapter: surface error.message and guard pingTimeout cleanup - urls: fix typo and correct JSDoc @param names Co-authored-by: Claude Opus 4.7 --- app/components/Modal.tsx | 17 ++++++------ app/scenes/Document/components/Header.tsx | 32 +++++++++++------------ server/storage/redis.ts | 14 +++++++--- shared/utils/urls.ts | 6 ++--- 4 files changed, 37 insertions(+), 32 deletions(-) diff --git a/app/components/Modal.tsx b/app/components/Modal.tsx index 3e5c0cc164..01c2e98872 100644 --- a/app/components/Modal.tsx +++ b/app/components/Modal.tsx @@ -15,7 +15,6 @@ import usePrevious from "~/hooks/usePrevious"; import { fadeAndScaleIn, fadeIn } from "~/styles/animations"; import Desktop from "~/utils/Desktop"; import ErrorBoundary from "./ErrorBoundary"; -import * as VisuallyHidden from "@radix-ui/react-visually-hidden"; import Tooltip from "./Tooltip"; import { useDialogContext } from "~/components/DialogContext"; @@ -32,7 +31,7 @@ type Props = { const Modal: React.FC = ({ children, isOpen, - title = "Untitled", + title, style, width, height, @@ -41,6 +40,7 @@ const Modal: React.FC = ({ const wasOpen = usePrevious(isOpen); const isMobile = useMobile(); const { t } = useTranslation(); + const resolvedTitle = title ?? t("Untitled"); const dialog = useDialogContext(); const onClose = React.useCallback(() => { @@ -56,9 +56,6 @@ const Modal: React.FC = ({ !open && onClose()}> - - {title} - = ({ ev.stopPropagation()} column> - {title && ( + - {title} + {resolvedTitle} - )} + {children} @@ -102,7 +99,9 @@ const Modal: React.FC = ({ {children}
- {title && {title}} + + {resolvedTitle} + diff --git a/app/scenes/Document/components/Header.tsx b/app/scenes/Document/components/Header.tsx index 7759df38fe..2f12b3e1f6 100644 --- a/app/scenes/Document/components/Header.tsx +++ b/app/scenes/Document/components/Header.tsx @@ -3,13 +3,13 @@ import { TableOfContentsIcon, EditIcon } from "outline-icons"; import { useState, useCallback, useEffect } from "react"; import { useTranslation } from "react-i18next"; import { Link } from "react-router-dom"; +import useMeasure from "react-use-measure"; import styled, { useTheme } from "styled-components"; import Icon from "@shared/components/Icon"; -import useMeasure from "react-use-measure"; +import useShare from "@shared/hooks/useShare"; import { altDisplay, metaDisplay } from "@shared/utils/keyboard"; -import type Document from "~/models/Document"; -import type Revision from "~/models/Revision"; -import type Template from "~/models/Template"; +import { publishDocument } from "~/actions/definitions/documents"; +import { restoreRevision } from "~/actions/definitions/revisions"; import { Action, Separator } from "~/components/Actions"; import Badge from "~/components/Badge"; import Button from "~/components/Button"; @@ -18,10 +18,14 @@ import DocumentBreadcrumb from "~/components/DocumentBreadcrumb"; import { useDocumentContext } from "~/components/DocumentContext"; import Flex from "~/components/Flex"; import Header from "~/components/Header"; +import { + AppearanceAction, + SubscribeAction, +} from "~/components/Sharing/components/Actions"; import Star from "~/components/Star"; import Tooltip from "~/components/Tooltip"; -import { publishDocument } from "~/actions/definitions/documents"; -import { restoreRevision } from "~/actions/definitions/revisions"; +import { type Editor } from "~/editor"; +import env from "~/env"; import useCurrentTeam from "~/hooks/useCurrentTeam"; import useCurrentUser from "~/hooks/useCurrentUser"; import useEditingFocus from "~/hooks/useEditingFocus"; @@ -34,18 +38,14 @@ import DocumentMenu from "~/menus/DocumentMenu"; import NewChildDocumentMenu from "~/menus/NewChildDocumentMenu"; import TableOfContentsMenu from "~/menus/TableOfContentsMenu"; import TemplatesMenu from "~/menus/TemplatesMenu"; -import env from "~/env"; +import type Document from "~/models/Document"; +import type Revision from "~/models/Revision"; +import type Template from "~/models/Template"; import { documentEditPath } from "~/utils/routeHelpers"; +import { ChangesNavigation } from "./ChangesNavigation"; import ObservingBanner from "./ObservingBanner"; import PublicBreadcrumb from "./PublicBreadcrumb"; import ShareButton from "./ShareButton"; -import { - AppearanceAction, - SubscribeAction, -} from "~/components/Sharing/components/Actions"; -import useShare from "@shared/hooks/useShare"; -import { type Editor } from "~/editor"; -import { ChangesNavigation } from "./ChangesNavigation"; type Props = { editorRef: React.RefObject; @@ -102,7 +102,7 @@ function DocumentHeader({ // We cache this value for as long as the component is mounted so that if you // apply a template there is still the option to replace it until the user // navigates away from the doc - const [isNew] = useState(document.isPersistedOnce); + const [wasNew] = useState(document.isPersistedOnce); const handleSave = useCallback(() => { onSave({ @@ -273,7 +273,7 @@ function DocumentHeader({ limit={isCompact ? 3 : undefined} /> )} - {(isEditing || !user?.separateEditMode) && isNew && can.update && ( + {(isEditing || !user?.separateEditMode) && wasNew && can.update && ( { pingTimeout = setTimeout( () => reject(new Error("ping timeout")), @@ -106,7 +108,11 @@ export default class RedisAdapter extends Redis { }); this.disconnect(true); }) - .finally(() => clearTimeout(pingTimeout)); + .finally(() => { + if (pingTimeout) { + clearTimeout(pingTimeout); + } + }); }, env.REDIS_HEALTHCHECK_INTERVAL); this.on("end", () => clearInterval(healthcheck)); diff --git a/shared/utils/urls.ts b/shared/utils/urls.ts index 927a26a33d..08efdf9b05 100644 --- a/shared/utils/urls.ts +++ b/shared/utils/urls.ts @@ -59,9 +59,9 @@ export function isInternalUrl(href: string) { } /** - * Returns true if the given string is a link to a documement. + * Returns true if the given string is a link to a document. * - * @param options Parsing options. + * @param url The url to check. * @returns True if a document, false otherwise. */ export function isDocumentUrl(url: string) { @@ -79,7 +79,7 @@ export function isDocumentUrl(url: string) { /** * Returns true if the given string is a link to a collection. * - * @param options Parsing options. + * @param url The url to check. * @returns True if a collection, false otherwise. */ export function isCollectionUrl(url: string) {