Skip to content

Commit 897c72e

Browse files
authored
Fix tool panel scrolling so the action button stays reachable (#7688)
# 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 (0a50e76, 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.
1 parent 732ef18 commit 897c72e

4 files changed

Lines changed: 79 additions & 43 deletions

File tree

frontend/editor/src/core/components/tools/shared/ReviewToolStep.tsx

Lines changed: 48 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,25 @@ import { saveOperationResults } from "@app/services/operationResultsSaveService"
1414
import { useFileActions, useFileSelectors } from "@app/contexts/FileContext";
1515
import i18n from "@app/i18n";
1616

17+
/**
18+
* Nearest scrolling ancestor - in the right rail that is the tool panel's
19+
* ScrollArea viewport, whose overflow is `scroll`, not `auto`.
20+
*/
21+
function findScrollParent(element: HTMLElement): HTMLElement | null {
22+
let node = element.parentElement;
23+
while (node) {
24+
const { overflowY } = getComputedStyle(node);
25+
if (
26+
/(auto|scroll|overlay)/.test(overflowY) &&
27+
node.scrollHeight > node.clientHeight
28+
) {
29+
return node;
30+
}
31+
node = node.parentElement;
32+
}
33+
return null;
34+
}
35+
1736
export interface ReviewToolStepProps<TParams = unknown> {
1837
isVisible: boolean;
1938
operation: ToolOperationHook<TParams>;
@@ -81,26 +100,37 @@ function ReviewStepContent<TParams = unknown>({
81100
}
82101
};
83102

84-
// Auto-scroll to bottom when content appears
103+
// Reveal the results when they appear, or the download button lands below the
104+
// fold behind a tall settings step and reads as missing.
85105
useEffect(() => {
86-
if (
87-
stepRef.current &&
88-
(previewFiles.length > 0 ||
89-
operation.downloadUrl ||
90-
operation.errorMessage)
91-
) {
92-
const scrollableContainer = stepRef.current.closest(
93-
'[style*="overflow: auto"]',
94-
) as HTMLElement;
95-
if (scrollableContainer) {
96-
setTimeout(() => {
97-
scrollableContainer.scrollTo({
98-
top: scrollableContainer.scrollHeight,
99-
behavior: "smooth",
100-
});
101-
}, 100); // Small delay to ensure content is rendered
106+
const hasContent =
107+
previewFiles.length > 0 ||
108+
operation.downloadUrl ||
109+
operation.errorMessage;
110+
if (!stepRef.current || !hasContent) return;
111+
112+
// Small delay so the step has been laid out before it is measured.
113+
const timer = setTimeout(() => {
114+
const step = stepRef.current;
115+
const scroller = step && findScrollParent(step);
116+
if (!step || !scroller) return;
117+
118+
const stepRect = step.getBoundingClientRect();
119+
const viewRect = scroller.getBoundingClientRect();
120+
// Move the least that brings the step into view, and only ever the panel
121+
// itself - scrollIntoView() drags every ancestor and unpins the header.
122+
const delta = Math.min(
123+
stepRect.top - viewRect.top,
124+
stepRect.bottom - viewRect.bottom,
125+
);
126+
if (delta > 1) {
127+
scroller.scrollTo({
128+
top: scroller.scrollTop + delta,
129+
behavior: "smooth",
130+
});
102131
}
103-
}
132+
}, 100);
133+
return () => clearTimeout(timer);
104134
}, [previewFiles.length, operation.downloadUrl, operation.errorMessage]);
105135

106136
return (
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
/* The tool panel scrolls as a single column, so a tall settings step (PDF/UA is
2+
the worst offender) pushes the primary action below the fold with nothing to
3+
say it is there. Pinning keeps it reachable; when the flow already fits,
4+
sticky is inert and nothing moves. */
5+
.executeFooter {
6+
position: sticky;
7+
bottom: 0;
8+
z-index: 2;
9+
background: var(--c-surface, var(--mantine-color-body));
10+
display: flex;
11+
flex-direction: column;
12+
gap: var(--mantine-spacing-sm);
13+
/* Bleed across the flow's own padding so content cannot scroll through the
14+
gutters beside the button. The margin cancels the padding, so an unpinned
15+
footer still sits exactly where it did. */
16+
margin-inline: calc(var(--mantine-spacing-sm) * -1);
17+
padding-inline: var(--mantine-spacing-sm);
18+
/* Paint-only skirt covering the strip below the button once pinned; a padding
19+
here would change the resting layout. */
20+
box-shadow: 0 var(--mantine-spacing-sm) 0 0
21+
var(--c-surface, var(--mantine-color-body));
22+
}

frontend/editor/src/core/components/tools/shared/createToolFlow.tsx

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
import { StirlingFile } from "@app/types/fileContext";
1414
import type { TooltipTip } from "@app/types/tips";
1515
import type { ExecuteDisabledReason } from "@app/hooks/tools/shared/toolOperationTypes";
16+
import classes from "@app/components/tools/shared/createToolFlow.module.css";
1617

1718
export interface FilesStepConfig {
1819
selectedFiles: StirlingFile[];
@@ -152,8 +153,14 @@ export function createToolFlow<TParams = unknown>(
152153
: eb.paramsValid === false
153154
? "invalidParams"
154155
: null;
156+
// Pin the action only while it is the last thing in the flow; with a
157+
// review below it, a sticky footer would float over the results.
155158
return (
156-
<>
159+
<div
160+
className={
161+
config.review.isVisible ? undefined : classes.executeFooter
162+
}
163+
>
157164
<ScopedOperationButton
158165
selectedFiles={config.files.selectedFiles ?? []}
159166
disableScopeHints={eb.disableScopeHints}
@@ -172,7 +179,7 @@ export function createToolFlow<TParams = unknown>(
172179
data-tour="run-button"
173180
/>
174181
{config.belowExecuteButton}
175-
</>
182+
</div>
176183
);
177184
})()}
178185

frontend/editor/src/core/tools/Convert.tsx

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ const Convert = ({ onPreviewFile, onComplete, onError }: BaseToolProps) => {
3636
});
3737
setSelectedFiles(matching.map((file) => file.fileId));
3838
};
39-
const scrollContainerRef = useRef<HTMLDivElement>(null);
4039

4140
const convertParams = useConvertParameters();
4241
const convertOperation = useConvertOperation(convertParams.parameters);
@@ -48,16 +47,6 @@ const Convert = ({ onPreviewFile, onComplete, onError }: BaseToolProps) => {
4847
const skipNextSelectionResetRef = useRef(false);
4948
const previousSelectionRef = useRef<string>("");
5049

51-
const scrollToBottom = () => {
52-
if (scrollContainerRef.current) {
53-
scrollContainerRef.current.scrollTo({
54-
top: scrollContainerRef.current.scrollHeight,
55-
behavior: "smooth",
56-
});
57-
}
58-
};
59-
60-
const hasFiles = selectedFiles.length > 0;
6150
const hasResults =
6251
convertOperation.files.length > 0 ||
6352
convertOperation.downloadUrl !== null ||
@@ -115,18 +104,6 @@ const Convert = ({ onPreviewFile, onComplete, onError }: BaseToolProps) => {
115104
convertParams.parameters.toExtension,
116105
]);
117106

118-
useEffect(() => {
119-
if (hasFiles) {
120-
setTimeout(scrollToBottom, 100);
121-
}
122-
}, [hasFiles]);
123-
124-
useEffect(() => {
125-
if (hasResults) {
126-
setTimeout(scrollToBottom, 100);
127-
}
128-
}, [hasResults]);
129-
130107
const handleConvert = async () => {
131108
try {
132109
await convertOperation.executeOperation(

0 commit comments

Comments
 (0)