From 897c72e9d900a8f5366a77f343e3bd4cdf427d72 Mon Sep 17 00:00:00 2001 From: Anthony Stirling <77850077+Frooodle@users.noreply.github.com> Date: Thu, 27 Aug 2026 11:55:31 +0000 Subject: [PATCH] Fix tool panel scrolling so the action button stays reachable (#7688) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description of Changes After ui rework all scrolling in all tool panels stopped working This fixes this to allow tool panels to be scrollabe again ## What was wrong PDF/UA is the only convert target whose settings panel overflows the tool rail. Measured at 1920×1080: overflow was 0px for pdfa, pdfx, png, docx, epub, and 158px for pdfua. Its action button sat at bottom: 1220 in a 1080px viewport — 140px below the fold — and the info alert was clipped mid-sentence. The panel could be scrolled, but nothing said so (Mantine's scrollbar auto-hides). Normally the app would scroll the button into view for you. It didn't, because both mechanisms built to do that were dead ## Cause: Two separate mechanisms, both broken since the same commit (0a50e765b7, frontend editor restructure, 2026-05-22): 1. ReviewToolStep - shared by all 47 tools. It looked for its scroll container with: stepRef.current.closest('[style*="overflow: auto"]') Mantine's ScrollArea viewport sets inline overflow: scroll, not auto. I measured it live - closest() returns null, and document.querySelectorAll('[style*="overflow: auto"]') finds exactly 1 element anywhere in the page, and it isn't an ancestor of the panel. So the lookup silently found nothing and the scrollTo never ran, for every tool. 2. Convert.tsx - Convert only. It declared scrollContainerRef and a scrollToBottom() wired to two useEffects, but the ref was never attached to any element - createToolFlow() builds the JSX and no ref is passed through. Always null, so both effects were no-ops. Nothing else in the codebase has this pattern - I grepped for other closest('[style*="overflow…"]') lookups and other scrollToBottom/scrollContainerRef uses and both came back empty. ## The fix createToolFlow.module.css (new) + createToolFlow.tsx:156 — the execute button gets a position: sticky; bottom: 0 footer, the house pattern already used by FormFill.module.css. Applied only when the review step isn't visible, so it can never float over results. Sticky is inert when content fits, so the other 46 tools are untouched. ReviewToolStep.tsx:21 — real findScrollParent() walk replacing the broken selector, scrolling by the minimum delta needed and only the panel itself (never scrollIntoView(), which drags every ancestor). Also added the missing clearTimeout cleanup. Convert.tsx — deleted the dead ref and its two effects. --- ## Checklist ### General - [ ] I have read the [Contribution Guidelines](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/CONTRIBUTING.md) - [ ] I have read the [Stirling-PDF Developer Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/DeveloperGuide.md) (if applicable) - [ ] I have read the [How to add new languages to Stirling-PDF](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md) (if applicable) - [ ] I have performed a self-review of my own code - [ ] My changes generate no new warnings ### Documentation - [ ] I have updated relevant docs on [Stirling-PDF's doc repo](https://github.com/Stirling-Tools/Stirling-Tools.github.io/blob/main/docs/) (if functionality has heavily changed) - [ ] I have read the section [Add New Translation Tags](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md#add-new-translation-tags) (for new translation tags only) ### Translations (if applicable) - [ ] I ran [`scripts/counter_translation.py`](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/docs/counter_translation.md) ### UI Changes (if applicable) - [ ] Screenshots or videos demonstrating the UI changes are attached (e.g., as comments or direct attachments in the PR) ### Testing (if applicable) - [ ] I have run `task check` to verify linters, typechecks, and tests pass - [ ] I have tested my changes locally. Refer to the [Testing Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/DeveloperGuide.md#7-testing) for more details. --- .../tools/shared/ReviewToolStep.tsx | 66 ++++++++++++++----- .../tools/shared/createToolFlow.module.css | 22 +++++++ .../tools/shared/createToolFlow.tsx | 11 +++- frontend/editor/src/core/tools/Convert.tsx | 23 ------- 4 files changed, 79 insertions(+), 43 deletions(-) create mode 100644 frontend/editor/src/core/components/tools/shared/createToolFlow.module.css diff --git a/frontend/editor/src/core/components/tools/shared/ReviewToolStep.tsx b/frontend/editor/src/core/components/tools/shared/ReviewToolStep.tsx index ef596c2a0d..8f86faff17 100644 --- a/frontend/editor/src/core/components/tools/shared/ReviewToolStep.tsx +++ b/frontend/editor/src/core/components/tools/shared/ReviewToolStep.tsx @@ -14,6 +14,25 @@ import { saveOperationResults } from "@app/services/operationResultsSaveService" import { useFileActions, useFileSelectors } from "@app/contexts/FileContext"; import i18n from "@app/i18n"; +/** + * Nearest scrolling ancestor - in the right rail that is the tool panel's + * ScrollArea viewport, whose overflow is `scroll`, not `auto`. + */ +function findScrollParent(element: HTMLElement): HTMLElement | null { + let node = element.parentElement; + while (node) { + const { overflowY } = getComputedStyle(node); + if ( + /(auto|scroll|overlay)/.test(overflowY) && + node.scrollHeight > node.clientHeight + ) { + return node; + } + node = node.parentElement; + } + return null; +} + export interface ReviewToolStepProps { isVisible: boolean; operation: ToolOperationHook; @@ -81,26 +100,37 @@ function ReviewStepContent({ } }; - // Auto-scroll to bottom when content appears + // Reveal the results when they appear, or the download button lands below the + // fold behind a tall settings step and reads as missing. useEffect(() => { - if ( - stepRef.current && - (previewFiles.length > 0 || - operation.downloadUrl || - operation.errorMessage) - ) { - const scrollableContainer = stepRef.current.closest( - '[style*="overflow: auto"]', - ) as HTMLElement; - if (scrollableContainer) { - setTimeout(() => { - scrollableContainer.scrollTo({ - top: scrollableContainer.scrollHeight, - behavior: "smooth", - }); - }, 100); // Small delay to ensure content is rendered + const hasContent = + previewFiles.length > 0 || + operation.downloadUrl || + operation.errorMessage; + if (!stepRef.current || !hasContent) return; + + // Small delay so the step has been laid out before it is measured. + const timer = setTimeout(() => { + const step = stepRef.current; + const scroller = step && findScrollParent(step); + if (!step || !scroller) return; + + const stepRect = step.getBoundingClientRect(); + const viewRect = scroller.getBoundingClientRect(); + // Move the least that brings the step into view, and only ever the panel + // itself - scrollIntoView() drags every ancestor and unpins the header. + const delta = Math.min( + stepRect.top - viewRect.top, + stepRect.bottom - viewRect.bottom, + ); + if (delta > 1) { + scroller.scrollTo({ + top: scroller.scrollTop + delta, + behavior: "smooth", + }); } - } + }, 100); + return () => clearTimeout(timer); }, [previewFiles.length, operation.downloadUrl, operation.errorMessage]); return ( diff --git a/frontend/editor/src/core/components/tools/shared/createToolFlow.module.css b/frontend/editor/src/core/components/tools/shared/createToolFlow.module.css new file mode 100644 index 0000000000..bc7abb630d --- /dev/null +++ b/frontend/editor/src/core/components/tools/shared/createToolFlow.module.css @@ -0,0 +1,22 @@ +/* The tool panel scrolls as a single column, so a tall settings step (PDF/UA is + the worst offender) pushes the primary action below the fold with nothing to + say it is there. Pinning keeps it reachable; when the flow already fits, + sticky is inert and nothing moves. */ +.executeFooter { + position: sticky; + bottom: 0; + z-index: 2; + background: var(--c-surface, var(--mantine-color-body)); + display: flex; + flex-direction: column; + gap: var(--mantine-spacing-sm); + /* Bleed across the flow's own padding so content cannot scroll through the + gutters beside the button. The margin cancels the padding, so an unpinned + footer still sits exactly where it did. */ + margin-inline: calc(var(--mantine-spacing-sm) * -1); + padding-inline: var(--mantine-spacing-sm); + /* Paint-only skirt covering the strip below the button once pinned; a padding + here would change the resting layout. */ + box-shadow: 0 var(--mantine-spacing-sm) 0 0 + var(--c-surface, var(--mantine-color-body)); +} diff --git a/frontend/editor/src/core/components/tools/shared/createToolFlow.tsx b/frontend/editor/src/core/components/tools/shared/createToolFlow.tsx index aafa9b3ea1..4b03de0a4b 100644 --- a/frontend/editor/src/core/components/tools/shared/createToolFlow.tsx +++ b/frontend/editor/src/core/components/tools/shared/createToolFlow.tsx @@ -13,6 +13,7 @@ import { import { StirlingFile } from "@app/types/fileContext"; import type { TooltipTip } from "@app/types/tips"; import type { ExecuteDisabledReason } from "@app/hooks/tools/shared/toolOperationTypes"; +import classes from "@app/components/tools/shared/createToolFlow.module.css"; export interface FilesStepConfig { selectedFiles: StirlingFile[]; @@ -152,8 +153,14 @@ export function createToolFlow( : eb.paramsValid === false ? "invalidParams" : null; + // Pin the action only while it is the last thing in the flow; with a + // review below it, a sticky footer would float over the results. return ( - <> +
( data-tour="run-button" /> {config.belowExecuteButton} - +
); })()} diff --git a/frontend/editor/src/core/tools/Convert.tsx b/frontend/editor/src/core/tools/Convert.tsx index eb9a4a02c5..733a21b6e4 100644 --- a/frontend/editor/src/core/tools/Convert.tsx +++ b/frontend/editor/src/core/tools/Convert.tsx @@ -36,7 +36,6 @@ const Convert = ({ onPreviewFile, onComplete, onError }: BaseToolProps) => { }); setSelectedFiles(matching.map((file) => file.fileId)); }; - const scrollContainerRef = useRef(null); const convertParams = useConvertParameters(); const convertOperation = useConvertOperation(convertParams.parameters); @@ -48,16 +47,6 @@ const Convert = ({ onPreviewFile, onComplete, onError }: BaseToolProps) => { const skipNextSelectionResetRef = useRef(false); const previousSelectionRef = useRef(""); - const scrollToBottom = () => { - if (scrollContainerRef.current) { - scrollContainerRef.current.scrollTo({ - top: scrollContainerRef.current.scrollHeight, - behavior: "smooth", - }); - } - }; - - const hasFiles = selectedFiles.length > 0; const hasResults = convertOperation.files.length > 0 || convertOperation.downloadUrl !== null || @@ -115,18 +104,6 @@ const Convert = ({ onPreviewFile, onComplete, onError }: BaseToolProps) => { convertParams.parameters.toExtension, ]); - useEffect(() => { - if (hasFiles) { - setTimeout(scrollToBottom, 100); - } - }, [hasFiles]); - - useEffect(() => { - if (hasResults) { - setTimeout(scrollToBottom, 100); - } - }, [hasResults]); - const handleConvert = async () => { try { await convertOperation.executeOperation(