fix: export video CSP preview, code panel UX, and output dock visibility - #191
Merged
Conversation
…onality and UI improvements
✅ Deploy Preview for dev-bayanflow ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
github-actions
Bot
added
style
📝 WalkthroughWalkthroughAdds Run/Reset controls to the output console, refactors the Python panel layout, locks body scrolling when panels open, updates translations, and adds CSP parsing and validation for the Remotion origin. ChangesOutput panel and page behavior
CSP video-export enforcement
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: dependency version conflict. Check your lock file or package.json. Comment |
… and PythonCodePanel components
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/cspHeaders.js`:
- Around line 64-75: The CSP validation in cspHeaders.js is matching sources
with substring checks, so values like a longer token can incorrectly satisfy the
guard. Update the mediaSrc and connectSrc validation to parse each directive
into individual tokens and check exact membership for 'self', blob:, and
https://www.remotion.pro rather than using includes() on the full string. Keep
the existing error messages and source context, but ensure the logic only passes
when the required CSP source is present as a distinct token.
In `@src/components/OutputConsole.jsx`:
- Line 179: The only issue here is trailing whitespace in the OutputConsole
component. Remove the extra spaces on the blank/indented line in
OutputConsole.jsx so the formatting passes quality checks, and keep the
surrounding JSX/markup unchanged.
- Around line 90-97: The run button label in OutputConsole should not depend on
output or error presence, since successful no-stdout executions still need to
show the post-run action. Update the runLabel logic in OutputConsole.jsx to base
the Run/Rerun text on execution state from status, or an explicit hasRun flag if
available, so completed runs always show Rerun even when output and error are
empty. Keep the loading/running branches unchanged and adjust the final fallback
to reflect whether the console has already executed.
In `@src/components/PythonCodePanel.jsx`:
- Around line 596-629: The collapse/expand logic in PythonCodePanel is
remounting OutputConsole and resetting its internal tab state. Update the
OutputConsole usage so there is only one mounted instance across
expanded/collapsed states, or lift the tab state out of OutputConsole into
PythonCodePanel and pass it through props. Keep the fix centered on the
OutputConsole render paths and the isOutputExpanded toggle behavior so the
selected tab and other dock state persist after collapsing and re-expanding.
- Around line 499-516: The direction override is applied too broadly in
PythonCodePanel, causing OutputConsole and TestCasesPanel to inherit LTR and
lose RTL layout. Move the dir="ltr" override so it only wraps the Monaco editor
container, and leave the surrounding flex stack to inherit the app’s direction.
Use the existing PythonCodePanel structure and the editor container around the
code area to scope the change without affecting the output panels.
In `@src/components/PythonCodePanel.test.jsx`:
- Around line 703-709: The current assertion in PythonCodePanel.test.jsx is too
broad and can miss the toolbar regression because tablist.nextElementSibling may
still contain a standalone Run row. Update the test around the
tablist/editor/output assertions to explicitly verify that any Run button is
only rendered inside the output-console area, using the existing tablist,
monaco-editor, and output-console selectors to locate the elements.
In `@src/hooks/useBodyScrollLock.js`:
- Around line 7-44: Switch useBodyScrollLock from useEffect to useLayoutEffect
so the body style changes in the useBodyScrollLock hook are applied before paint
and avoid a scrollbar/layout flash. Update the React import in useBodyScrollLock
and keep the existing lock/unlock logic, including the document/window guards
and cleanup that restores body styles and scroll position.
In `@vite.config.js`:
- Around line 112-113: The CSP validation in the Vite build hook has a blind
spot when dist/_headers is missing, so the build can pass without running any
checks. Update the hook around extractCspFromHeadersFile and
assertVideoExportCspDirectives to explicitly verify that dist/_headers was
emitted before reading it, and fail the build with a clear error if it is
absent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3f288b01-a8cc-4060-acb7-2eed06b3ff9b
📒 Files selected for processing (16)
netlify.tomlpublic/_headersscripts/cspHeaders.jssrc/components/AlgorithmInsightPanel.jsxsrc/components/OutputConsole.jsxsrc/components/OutputConsole.test.jsxsrc/components/PythonCodePanel.jsxsrc/components/PythonCodePanel.test.jsxsrc/hooks/useBodyScrollLock.jssrc/hooks/useBodyScrollLock.test.jssrc/i18n/locales/ar/translation.jsonsrc/i18n/locales/en/translation.jsonsrc/i18n/locales/fr/translation.jsonsrc/pages/VisualizerApp.jsxsrc/security/cspHeaders.test.jsvite.config.js
Comment on lines
+64
to
+75
| const mediaSrc = directives.get('media-src'); | ||
| if (!mediaSrc?.includes("'self'") || !mediaSrc.includes('blob:')) { | ||
| throw new Error( | ||
| `${source}: media-src must include 'self' and blob: (required for export preview)` | ||
| ); | ||
| } | ||
|
|
||
| const connectSrc = directives.get('connect-src'); | ||
| if (!connectSrc?.includes('https://www.remotion.pro')) { | ||
| throw new Error( | ||
| `${source}: connect-src must include https://www.remotion.pro (Remotion telemetry)` | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Match CSP sources by token, not substring.
These includes() checks can pass on the wrong source expression, so the new build/test guard can report success even when the required CSP source is not actually present.
Suggested fix
export function assertVideoExportCspDirectives(csp, source) {
const directives = parseCspDirectives(csp);
+ const tokens = (value) => (value ? value.split(/\s+/).filter(Boolean) : []);
const mediaSrc = directives.get('media-src');
- if (!mediaSrc?.includes("'self'") || !mediaSrc.includes('blob:')) {
+ const mediaTokens = tokens(mediaSrc);
+ if (!mediaTokens.includes("'self'") || !mediaTokens.includes('blob:')) {
throw new Error(
`${source}: media-src must include 'self' and blob: (required for export preview)`
);
}
const connectSrc = directives.get('connect-src');
- if (!connectSrc?.includes('https://www.remotion.pro')) {
+ const connectTokens = tokens(connectSrc);
+ if (!connectTokens.includes('https://www.remotion.pro')) {
throw new Error(
`${source}: connect-src must include https://www.remotion.pro (Remotion telemetry)`
);
}
return { mediaSrc, connectSrc: connectSrc ?? '' };
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const mediaSrc = directives.get('media-src'); | |
| if (!mediaSrc?.includes("'self'") || !mediaSrc.includes('blob:')) { | |
| throw new Error( | |
| `${source}: media-src must include 'self' and blob: (required for export preview)` | |
| ); | |
| } | |
| const connectSrc = directives.get('connect-src'); | |
| if (!connectSrc?.includes('https://www.remotion.pro')) { | |
| throw new Error( | |
| `${source}: connect-src must include https://www.remotion.pro (Remotion telemetry)` | |
| ); | |
| const mediaSrc = directives.get('media-src'); | |
| const tokens = (value) => (value ? value.split(/\s+/).filter(Boolean) : []); | |
| const mediaTokens = tokens(mediaSrc); | |
| if (!mediaTokens.includes("'self'") || !mediaTokens.includes('blob:')) { | |
| throw new Error( | |
| `${source}: media-src must include 'self' and blob: (required for export preview)` | |
| ); | |
| } | |
| const connectSrc = directives.get('connect-src'); | |
| const connectTokens = tokens(connectSrc); | |
| if (!connectTokens.includes('https://www.remotion.pro')) { | |
| throw new Error( | |
| `${source}: connect-src must include https://www.remotion.pro (Remotion telemetry)` | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/cspHeaders.js` around lines 64 - 75, The CSP validation in
cspHeaders.js is matching sources with substring checks, so values like a longer
token can incorrectly satisfy the guard. Update the mediaSrc and connectSrc
validation to parse each directive into individual tokens and check exact
membership for 'self', blob:, and https://www.remotion.pro rather than using
includes() on the full string. Keep the existing error messages and source
context, but ensure the logic only passes when the required CSP source is
present as a distinct token.
Comment on lines
+90
to
+97
| const runLabel = | ||
| status === 'loading' | ||
| ? t('python_code.loading_runtime', { defaultValue: 'Loading Python...' }) | ||
| : status === 'running' | ||
| ? t('python_code.running', { defaultValue: 'Running...' }) | ||
| : output || error | ||
| ? t('python_code.rerun', { defaultValue: 'Rerun' }) | ||
| : t('python_code.run', { defaultValue: 'Run' }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Base the Run/Rerun label on execution state, not console content.
A successful run with no stdout leaves both output and error empty, so this branch falls back to "Run" even after execution completed. That makes the new action text inconsistent for valid no-output programs. Key it off terminal status (or an explicit hasRun flag) instead.
Suggested fix
- const runLabel =
- status === 'loading'
- ? t('python_code.loading_runtime', { defaultValue: 'Loading Python...' })
- : status === 'running'
- ? t('python_code.running', { defaultValue: 'Running...' })
- : output || error
- ? t('python_code.rerun', { defaultValue: 'Rerun' })
- : t('python_code.run', { defaultValue: 'Run' });
+ const hasRun =
+ status === 'success' || status === 'error' || status === 'timeout';
+
+ const runLabel =
+ status === 'loading'
+ ? t('python_code.loading_runtime', { defaultValue: 'Loading Python...' })
+ : status === 'running'
+ ? t('python_code.running', { defaultValue: 'Running...' })
+ : hasRun
+ ? t('python_code.rerun', { defaultValue: 'Rerun' })
+ : t('python_code.run', { defaultValue: 'Run' });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const runLabel = | |
| status === 'loading' | |
| ? t('python_code.loading_runtime', { defaultValue: 'Loading Python...' }) | |
| : status === 'running' | |
| ? t('python_code.running', { defaultValue: 'Running...' }) | |
| : output || error | |
| ? t('python_code.rerun', { defaultValue: 'Rerun' }) | |
| : t('python_code.run', { defaultValue: 'Run' }); | |
| const hasRun = | |
| status === 'success' || status === 'error' || status === 'timeout'; | |
| const runLabel = | |
| status === 'loading' | |
| ? t('python_code.loading_runtime', { defaultValue: 'Loading Python...' }) | |
| : status === 'running' | |
| ? t('python_code.running', { defaultValue: 'Running...' }) | |
| : hasRun | |
| ? t('python_code.rerun', { defaultValue: 'Rerun' }) | |
| : t('python_code.run', { defaultValue: 'Run' }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/OutputConsole.jsx` around lines 90 - 97, The run button label
in OutputConsole should not depend on output or error presence, since successful
no-stdout executions still need to show the post-run action. Update the runLabel
logic in OutputConsole.jsx to base the Run/Rerun text on execution state from
status, or an explicit hasRun flag if available, so completed runs always show
Rerun even when output and error are empty. Keep the loading/running branches
unchanged and adjust the final fallback to reflect whether the console has
already executed.
Comment on lines
+499
to
+516
| <div | ||
| ref={resizeContainerRef} | ||
| className="flex-1 min-h-0 flex flex-col overflow-hidden relative" | ||
| className="flex-1 min-h-0 flex flex-col p-2" | ||
| dir="ltr" | ||
| > | ||
| <div | ||
| className={`min-h-0 overflow-hidden ${isMobile ? 'touch-pan-y' : ''}`} | ||
| style={{ | ||
| flex: | ||
| isMobile || !isOutputExpanded | ||
| ? '1 1 0' | ||
| : `0 0 calc(${100 - outputHeightPercent}% - 2px)`, | ||
| minHeight: isMobile ? 100 : 80, | ||
| }} | ||
| ref={resizeContainerRef} | ||
| className="min-h-0 flex flex-col overflow-hidden flex-1" | ||
| > | ||
| <div | ||
| className={`min-h-0 overflow-hidden ${isMobile ? 'touch-pan-y' : ''}`} | ||
| style={{ | ||
| flex: | ||
| isMobile || !isOutputExpanded | ||
| ? '1 1 0' | ||
| : `0 0 calc(${100 - outputHeightPercent}% - 2px)`, | ||
| minHeight: isMobile ? 100 : 80, | ||
| }} | ||
| > |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Don't force the whole editor/output stack to LTR.
Line 501 now makes OutputConsole and TestCasesPanel inherit dir="ltr", so Arabic dock labels/layout lose RTL behavior. Scope the direction override to the Monaco container only.
Suggested fix
- <div
- className="flex-1 min-h-0 flex flex-col p-2"
- dir="ltr"
- >
+ <div className="flex-1 min-h-0 flex flex-col p-2">
<div
ref={resizeContainerRef}
className="min-h-0 flex flex-col overflow-hidden flex-1"
>
<div
+ dir="ltr"
className={`min-h-0 overflow-hidden ${isMobile ? 'touch-pan-y' : ''}`}
style={{📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <div | |
| ref={resizeContainerRef} | |
| className="flex-1 min-h-0 flex flex-col overflow-hidden relative" | |
| className="flex-1 min-h-0 flex flex-col p-2" | |
| dir="ltr" | |
| > | |
| <div | |
| className={`min-h-0 overflow-hidden ${isMobile ? 'touch-pan-y' : ''}`} | |
| style={{ | |
| flex: | |
| isMobile || !isOutputExpanded | |
| ? '1 1 0' | |
| : `0 0 calc(${100 - outputHeightPercent}% - 2px)`, | |
| minHeight: isMobile ? 100 : 80, | |
| }} | |
| ref={resizeContainerRef} | |
| className="min-h-0 flex flex-col overflow-hidden flex-1" | |
| > | |
| <div | |
| className={`min-h-0 overflow-hidden ${isMobile ? 'touch-pan-y' : ''}`} | |
| style={{ | |
| flex: | |
| isMobile || !isOutputExpanded | |
| ? '1 1 0' | |
| : `0 0 calc(${100 - outputHeightPercent}% - 2px)`, | |
| minHeight: isMobile ? 100 : 80, | |
| }} | |
| > | |
| <div className="flex-1 min-h-0 flex flex-col p-2"> | |
| <div | |
| ref={resizeContainerRef} | |
| className="min-h-0 flex flex-col overflow-hidden flex-1" | |
| > | |
| <div | |
| dir="ltr" | |
| className={`min-h-0 overflow-hidden ${isMobile ? 'touch-pan-y' : ''}`} | |
| style={{ | |
| flex: | |
| isMobile || !isOutputExpanded | |
| ? '1 1 0' | |
| : `0 0 calc(${100 - outputHeightPercent}% - 2px)`, | |
| minHeight: isMobile ? 100 : 80, | |
| }} | |
| > |
🧰 Tools
🪛 GitHub Check: Code Quality
[failure] 499-499:
Replace ⏎····················className="flex-1·min-h-0·flex·flex-col·p-2"⏎····················dir="ltr"⏎·················· with ·className="flex-1·min-h-0·flex·flex-col·p-2"·dir="ltr"
🪛 GitHub Check: Upload PR preview
[failure] 499-499:
Replace ⏎····················className="flex-1·min-h-0·flex·flex-col·p-2"⏎····················dir="ltr"⏎·················· with ·className="flex-1·min-h-0·flex·flex-col·p-2"·dir="ltr"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/PythonCodePanel.jsx` around lines 499 - 516, The direction
override is applied too broadly in PythonCodePanel, causing OutputConsole and
TestCasesPanel to inherit LTR and lose RTL layout. Move the dir="ltr" override
so it only wraps the Monaco editor container, and leave the surrounding flex
stack to inherit the app’s direction. Use the existing PythonCodePanel structure
and the editor container around the code area to scope the change without
affecting the output panels.
Comment on lines
+596
to
+629
| {isOutputExpanded && ( | ||
| <div | ||
| className="min-h-0 overflow-hidden flex flex-col shrink-0" | ||
| style={{ | ||
| flex: isMobile | ||
| ? '0 0 auto' | ||
| : `0 0 calc(${outputHeightPercent}% - 2px)`, | ||
| minHeight: | ||
| isOutputExpanded && !isMobile ? 80 : undefined, | ||
| }} | ||
| > | ||
| <OutputConsole | ||
| status={status} | ||
| output={output} | ||
| error={error} | ||
| onClear={clearOutput} | ||
| isExpanded={isOutputExpanded} | ||
| onToggleExpand={() => | ||
| setIsOutputExpanded(prev => !prev) | ||
| } | ||
| testCases={testCases} | ||
| testResults={testResults} | ||
| testStatus={testStatus} | ||
| testError={testError} | ||
| onRunTests={handleRunTests} | ||
| onAddTestCase={handleAddTestCase} | ||
| onEditTestCase={handleEditTestCase} | ||
| onDeleteTestCase={handleDeleteTestCase} | ||
| onClearTestResults={clearTestResults} | ||
| /> | ||
| minHeight: !isMobile ? 80 : undefined, | ||
| }} | ||
| > | ||
| <OutputConsole | ||
| status={status} | ||
| output={output} | ||
| error={error} | ||
| onClear={clearOutput} | ||
| isExpanded={isOutputExpanded} | ||
| onToggleExpand={() => | ||
| setIsOutputExpanded(prev => !prev) | ||
| } | ||
| onRun={handleRun} | ||
| onReset={() => setCode(pythonCode)} | ||
| isModified={isModified} | ||
| testCases={testCases} | ||
| testResults={testResults} | ||
| testStatus={testStatus} | ||
| testError={testError} | ||
| onRunTests={handleRunTests} | ||
| onAddTestCase={handleAddTestCase} | ||
| onEditTestCase={handleEditTestCase} | ||
| onDeleteTestCase={handleDeleteTestCase} | ||
| onClearTestResults={clearTestResults} | ||
| /> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Collapse/expand remounts the console and drops its tab state.
These branches mount different OutputConsole instances. Since OutputConsole keeps internalTab internally, collapsing while the user is on Test Cases re-expands back on Output, and any other uncontrolled dock state is lost. Keep a single instance or lift activeTab into PythonCodePanel.
Suggested fix
+ const [outputTab, setOutputTab] = useState('output');
+
{isOutputExpanded && (
<div
className="min-h-0 overflow-hidden flex flex-col shrink-0"
style={{
@@
<OutputConsole
+ activeTab={outputTab}
+ onTabChange={setOutputTab}
status={status}
output={output}
@@
<OutputConsole
+ activeTab={outputTab}
+ onTabChange={setOutputTab}
status={status}
output={output}Also applies to: 631-655
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] 615-615: Avoid using the initial state variable in setState
Context: setCode(pythonCode)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/PythonCodePanel.jsx` around lines 596 - 629, The
collapse/expand logic in PythonCodePanel is remounting OutputConsole and
resetting its internal tab state. Update the OutputConsole usage so there is
only one mounted instance across expanded/collapsed states, or lift the tab
state out of OutputConsole into PythonCodePanel and pass it through props. Keep
the fix centered on the OutputConsole render paths and the isOutputExpanded
toggle behavior so the selected tab and other dock state persist after
collapsing and re-expanding.
Comment on lines
+703
to
+709
| const tablist = screen.getByRole('tablist'); | ||
| const editor = screen.getByTestId('monaco-editor'); | ||
| const tablistParent = tablist.parentElement; | ||
| const editorContainer = editor.parentElement?.parentElement; | ||
|
|
||
| expect(tablistParent).toContainElement(editorContainer); | ||
| expect(tablist.nextElementSibling).not.toHaveTextContent(/^Run$/); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
This assertion won't catch the toolbar regression.
tablist.nextElementSibling is the whole editor/output wrapper, and not.toHaveTextContent(/^Run$/) only fails when that wrapper's entire text is exactly Run. A standalone Run row above the editor would still pass. Assert that the only Run button lives inside output-console instead.
Suggested fix
- expect(tablist.nextElementSibling).not.toHaveTextContent(/^Run$/);
+ const runButtons = screen.getAllByRole('button', { name: /^run$/i });
+ expect(runButtons).toHaveLength(1);
+ expect(screen.getByTestId('output-console')).toContainElement(
+ runButtons[0]
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const tablist = screen.getByRole('tablist'); | |
| const editor = screen.getByTestId('monaco-editor'); | |
| const tablistParent = tablist.parentElement; | |
| const editorContainer = editor.parentElement?.parentElement; | |
| expect(tablistParent).toContainElement(editorContainer); | |
| expect(tablist.nextElementSibling).not.toHaveTextContent(/^Run$/); | |
| const tablist = screen.getByRole('tablist'); | |
| const editor = screen.getByTestId('monaco-editor'); | |
| const tablistParent = tablist.parentElement; | |
| const editorContainer = editor.parentElement?.parentElement; | |
| expect(tablistParent).toContainElement(editorContainer); | |
| const runButtons = screen.getAllByRole('button', { name: /^run$/i }); | |
| expect(runButtons).toHaveLength(1); | |
| expect(screen.getByTestId('output-console')).toContainElement( | |
| runButtons[0] | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/PythonCodePanel.test.jsx` around lines 703 - 709, The current
assertion in PythonCodePanel.test.jsx is too broad and can miss the toolbar
regression because tablist.nextElementSibling may still contain a standalone Run
row. Update the test around the tablist/editor/output assertions to explicitly
verify that any Run button is only rendered inside the output-console area,
using the existing tablist, monaco-editor, and output-console selectors to
locate the elements.
Comment on lines
+7
to
+44
| import { useEffect } from 'react'; | ||
|
|
||
| /** | ||
| * Prevent the document from scrolling while an overlay (side panel, modal) is open. | ||
| * Preserves scroll position using position:fixed on body. | ||
| * | ||
| * @param {boolean} isLocked - Whether scroll should be locked | ||
| */ | ||
| export function useBodyScrollLock(isLocked) { | ||
| useEffect(() => { | ||
| if ( | ||
| !isLocked || | ||
| typeof window === 'undefined' || | ||
| typeof document === 'undefined' | ||
| ) { | ||
| return; | ||
| } | ||
|
|
||
| const scrollY = window.scrollY; | ||
| const { body } = document; | ||
| const previousPosition = body.style.position; | ||
| const previousTop = body.style.top; | ||
| const previousWidth = body.style.width; | ||
| const previousOverflow = body.style.overflow; | ||
|
|
||
| body.style.position = 'fixed'; | ||
| body.style.top = `-${scrollY}px`; | ||
| body.style.width = '100%'; | ||
| body.style.overflow = 'hidden'; | ||
|
|
||
| return () => { | ||
| body.style.position = previousPosition; | ||
| body.style.top = previousTop; | ||
| body.style.width = previousWidth; | ||
| body.style.overflow = previousOverflow; | ||
| window.scrollTo(0, scrollY); | ||
| }; | ||
| }, [isLocked]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## useBodyScrollLock.js\n'
sed -n '1,220p' src/hooks/useBodyScrollLock.js
printf '\n## VisualizerApp.jsx references\n'
rg -n "useBodyScrollLock|scroll lock|body scroll" src/pages/VisualizerApp.jsx src -g '!**/node_modules/**'
printf '\n## project markers\n'
git ls-files | rg '^(package.json|vite.config|next.config|remix.config|astro.config|gatsby-config|src/|app/|pages/|public/)' | sed -n '1,160p'Repository: ayoub3bidi/bayan-flow
Length of output: 8965
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## package.json\n'
sed -n '1,240p' package.json
printf '\n## app entry points\n'
git ls-files src | rg '(^src/(main|index)\.(jsx|js|tsx|ts)$|^src/App\.(jsx|js|tsx|ts)$|^src/pages/VisualizerApp\.jsx$|^src/components/.*\.(jsx|js|tsx|ts)$)' | sed -n '1,120p'
printf '\n## useLayoutEffect usages\n'
rg -n "useLayoutEffect" src package.json
printf '\n## Vite/SSR markers\n'
rg -n "createRoot|hydrateRoot|ReactDOM|SSR|server-side|useLayoutEffect" src vite.config.* package.jsonRepository: ayoub3bidi/bayan-flow
Length of output: 6682
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## src/main.jsx\n'
sed -n '1,220p' src/main.jsx
printf '\n## VisualizerApp.jsx around the hook call\n'
sed -n '120,170p' src/pages/VisualizerApp.jsxRepository: ayoub3bidi/bayan-flow
Length of output: 3504
Switch useBodyScrollLock to useLayoutEffect.
This writes layout-affecting body styles when the panel opens. useEffect can let one frame paint before the lock lands, which can cause a scrollbar/layout flash.
Proposed fix
-import { useEffect } from 'react';
+import { useLayoutEffect } from 'react';
@@
export function useBodyScrollLock(isLocked) {
- useEffect(() => {
+ useLayoutEffect(() => {
@@
- }, [isLocked]);
+ }, [isLocked]);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import { useEffect } from 'react'; | |
| /** | |
| * Prevent the document from scrolling while an overlay (side panel, modal) is open. | |
| * Preserves scroll position using position:fixed on body. | |
| * | |
| * @param {boolean} isLocked - Whether scroll should be locked | |
| */ | |
| export function useBodyScrollLock(isLocked) { | |
| useEffect(() => { | |
| if ( | |
| !isLocked || | |
| typeof window === 'undefined' || | |
| typeof document === 'undefined' | |
| ) { | |
| return; | |
| } | |
| const scrollY = window.scrollY; | |
| const { body } = document; | |
| const previousPosition = body.style.position; | |
| const previousTop = body.style.top; | |
| const previousWidth = body.style.width; | |
| const previousOverflow = body.style.overflow; | |
| body.style.position = 'fixed'; | |
| body.style.top = `-${scrollY}px`; | |
| body.style.width = '100%'; | |
| body.style.overflow = 'hidden'; | |
| return () => { | |
| body.style.position = previousPosition; | |
| body.style.top = previousTop; | |
| body.style.width = previousWidth; | |
| body.style.overflow = previousOverflow; | |
| window.scrollTo(0, scrollY); | |
| }; | |
| }, [isLocked]); | |
| import { useLayoutEffect } from 'react'; | |
| /** | |
| * Prevent the document from scrolling while an overlay (side panel, modal) is open. | |
| * Preserves scroll position using position:fixed on body. | |
| * | |
| * `@param` {boolean} isLocked - Whether scroll should be locked | |
| */ | |
| export function useBodyScrollLock(isLocked) { | |
| useLayoutEffect(() => { | |
| if ( | |
| !isLocked || | |
| typeof window === 'undefined' || | |
| typeof document === 'undefined' | |
| ) { | |
| return; | |
| } | |
| const scrollY = window.scrollY; | |
| const { body } = document; | |
| const previousPosition = body.style.position; | |
| const previousTop = body.style.top; | |
| const previousWidth = body.style.width; | |
| const previousOverflow = body.style.overflow; | |
| body.style.position = 'fixed'; | |
| body.style.top = `-${scrollY}px`; | |
| body.style.width = '100%'; | |
| body.style.overflow = 'hidden'; | |
| return () => { | |
| body.style.position = previousPosition; | |
| body.style.top = previousTop; | |
| body.style.width = previousWidth; | |
| body.style.overflow = previousOverflow; | |
| window.scrollTo(0, scrollY); | |
| }; | |
| }, [isLocked]); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hooks/useBodyScrollLock.js` around lines 7 - 44, Switch useBodyScrollLock
from useEffect to useLayoutEffect so the body style changes in the
useBodyScrollLock hook are applied before paint and avoid a scrollbar/layout
flash. Update the React import in useBodyScrollLock and keep the existing
lock/unlock logic, including the document/window guards and cleanup that
restores body styles and scroll position.
Comment on lines
+112
to
+113
| const csp = extractCspFromHeadersFile(content); | ||
| assertVideoExportCspDirectives(csp, 'dist/_headers'); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fail the build when dist/_headers is missing.
This validation still has a blind spot: if _headers is not emitted, the hook exits before these checks run. That means the build can pass with no CSP validation at all.
Suggested fix
closeBundle() {
const headersPath = path.join(__dirname, 'dist', '_headers');
if (!fs.existsSync(headersPath)) {
- return;
+ throw new Error('dist/_headers was not emitted; cannot validate CSP headers');
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const csp = extractCspFromHeadersFile(content); | |
| assertVideoExportCspDirectives(csp, 'dist/_headers'); | |
| if (!fs.existsSync(headersPath)) { | |
| throw new Error('dist/_headers was not emitted; cannot validate CSP headers'); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@vite.config.js` around lines 112 - 113, The CSP validation in the Vite build
hook has a blind spot when dist/_headers is missing, so the build can pass
without running any checks. Update the hook around extractCspFromHeadersFile and
assertVideoExportCspDirectives to explicitly verify that dist/_headers was
emitted before reading it, and fail the build with a clear error if it is
absent.
…anup in VisualizerApp component
Preview for Bayan Flow Staging ready!
Preview alias |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/VisualizerApp.test.jsx (1)
353-367: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winClear the hoisted
vi.fn()state before each test.vi.restoreAllMocks()only restores spies; it won’t clear the call history on the module-scopedvi.fn()mocks used here, so thetoHaveBeenCalledTimes(...)assertions can leak across cases. Addvi.clearAllMocks()inbeforeEachor clear those hoisted mocks explicitly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/VisualizerApp.test.jsx` around lines 353 - 367, Clear the hoisted vi.fn() mock state before each test in VisualizerApp.test.jsx, because vi.restoreAllMocks() in afterEach only resets spies and does not wipe call history on the module-scoped mocks. Update the test setup around beforeEach/afterEach to call vi.clearAllMocks() (or explicitly reset the hoisted mocks) so assertions like toHaveBeenCalledTimes stay isolated across cases, using the existing resetSoundManagerMock, fullScreenMock, and videoExporterMock setup as the place to keep the test state clean.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/pages/VisualizerApp.test.jsx`:
- Around line 353-367: Clear the hoisted vi.fn() mock state before each test in
VisualizerApp.test.jsx, because vi.restoreAllMocks() in afterEach only resets
spies and does not wipe call history on the module-scoped mocks. Update the test
setup around beforeEach/afterEach to call vi.clearAllMocks() (or explicitly
reset the hoisted mocks) so assertions like toHaveBeenCalledTimes stay isolated
across cases, using the existing resetSoundManagerMock, fullScreenMock, and
videoExporterMock setup as the place to keep the test state clean.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: aed40ac2-5073-41e7-b24f-c592d3c19882
📒 Files selected for processing (3)
src/pages/VisualizerApp.test.jsxsrc/test/setup.jssrc/test/soundManagerMock.js
This was referenced Jun 27, 2026
28 tasks
35 tasks
This was referenced Jul 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes two production UX bugs and polishes the Python code side panel. Exported MP4 downloads worked but in-app preview failed on deployed sites because CSP blocked
blob:URLs for<video>elements. The code panel had background scroll bleed while open, a collapsed output section that disappeared off-screen, and a Run toolbar that broke visual continuity with the tab switchers.What was done
Export video preview (CSP)
media-src 'self' blob:so the post-export<video>preview can load the rendered blob URLhttps://www.remotion.protoconnect-srcfor Remotion telemetry (removes CSP console noise)public/_headersandnetlify.tomlscripts/cspHeaders.jswith CI tests insrc/security/cspHeaders.test.jsdist/_headersat build time invite.config.jsCode panel UX
useBodyScrollLockinVisualizerAppwhile the Python or insight panel is open (preserves scroll position on close)overscroll-containonPythonCodePanelandAlgorithmInsightPanelOutputConsoleheader (Output / Test Cases dock); remove the standalone toolbar between tabs and Monacop-2gap between tab switchers and Monaco editortop-14 bottom-0instead ofh-full+top: 56px(panel was 56px taller than the viewport, hiding the bottom dock)overflow-hiddeneditor container so it stays pinned and visiblearia-labelfor accessibilityi18n
output_dock_label,expand_output,collapse_outputTechnical Details
media-src; browser fell back todefault-src 'self'and blockedblob:media-src 'self' blob:<a download>is not governed bymedia-srch-full+top: 56pxpushed panel bottom below viewport; flex +overflow-hiddenclipped collapsed headertop-14 bottom-0; collapsed dock as sibling below editor stackuseBodyScrollLockwith fixed body + scroll restorationMonaco Ctrl/Cmd+Enter run shortcut unchanged. Test Cases Run tests button in
TestCasesPanelunchanged (separate from code Run).Testing
pnpm vitest run src/security/cspHeaders.test.jspnpm vitest run src/hooks/useBodyScrollLock.test.jspnpm vitest run src/components/OutputConsole.test.jsx src/components/PythonCodePanel.test.jsxpnpm build(validatesdist/_headersCSP directives)Manual verification
Export (dev/staging with CSP headers):
blob:/media-srcCSP violationCode panel (
/app):Notes
pnpm devdoes not send CSP headers; export preview CSP fix applies on Cloudflare/Netlify builds onlyPR_DESCRIPTION.mdis for review handoff; exclude from commit unless you want it tracked in-repoRelated
blob:underdefault-srcfallback whenmedia-srcwas unsetSummary by CodeRabbit