OCPBUGS-74156: Prevent pod log search from shifting page layout#16223
Conversation
|
@sg00dwin: This pull request references Jira Issue OCPBUGS-74156, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@sg00dwin: This pull request references Jira Issue OCPBUGS-74156, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
📝 WalkthroughWalkthroughA ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/public/components/utils/resource-log.tsx (1)
815-827: Scope the scroll-reset listener to nowrap mode onlyThis listener is currently active for all log-view states. Since the bug occurs with wrap disabled, keep this effect gated by
!wrapLinesCheckboxso we avoid unnecessary global scroll interception and event churn.Proposed change
useEffect(() => { + if (wrapLinesCheckbox) { + return; + } const drawerMain = fullscreenRef.current?.closest('.pf-v6-c-drawer__main'); if (!drawerMain) { return; } const resetScroll = () => { if (drawerMain.scrollLeft !== 0) { drawerMain.scrollLeft = 0; } }; drawerMain.addEventListener('scroll', resetScroll); return () => drawerMain.removeEventListener('scroll', resetScroll); - }, [fullscreenRef]); + }, [fullscreenRef, wrapLinesCheckbox]);As per coding guidelines, "
**: -Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/public/components/utils/resource-log.tsx` around lines 815 - 827, The scroll-reset effect (useEffect) currently attaches resetScroll to drawerMain unconditionally; change it to run only when wrapLinesCheckbox is false by adding wrapLinesCheckbox to the dependency array and early-return if wrapLinesCheckbox is true; keep the existing resetScroll, add/remove listener logic tied to drawerMain, and ensure cleanup still removes the listener when wrapLinesCheckbox toggles or fullscreenRef changes (useEffect deps: [fullscreenRef, wrapLinesCheckbox]).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@frontend/public/components/utils/resource-log.tsx`:
- Around line 815-827: The scroll-reset effect (useEffect) currently attaches
resetScroll to drawerMain unconditionally; change it to run only when
wrapLinesCheckbox is false by adding wrapLinesCheckbox to the dependency array
and early-return if wrapLinesCheckbox is true; keep the existing resetScroll,
add/remove listener logic tied to drawerMain, and ensure cleanup still removes
the listener when wrapLinesCheckbox toggles or fullscreenRef changes (useEffect
deps: [fullscreenRef, wrapLinesCheckbox]).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: ff9fb777-115e-4195-9bf1-4e5e3216d78b
📒 Files selected for processing (1)
frontend/public/components/utils/resource-log.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
frontend/public/components/utils/resource-log.tsx
|
/retest-required |
|
@sg00dwin: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rhamilto, sg00dwin The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| // Workaround for upstream PF LogViewer bug: scrollIntoView({ inline: 'center' }) propagates | ||
| // to all scrollable ancestors, shifting the page layout when searching with wrap lines disabled. | ||
| // Reset scrollLeft on .pf-v6-c-drawer__main to prevent the page shift. | ||
| // https://github.com/patternfly/react-log-viewer/issues/106 |
There was a problem hiding this comment.
Can we contribute this upstream if a backport isn't needed?
|
@sg00dwin: Jira Issue Verification Checks: Jira Issue OCPBUGS-74156 Jira Issue OCPBUGS-74156 has been moved to the MODIFIED state and will move to the VERIFIED state when the change is available in an accepted nightly payload. 🕓 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Fix included in accepted release 4.22.0-0.nightly-2026-04-04-113332 |

Bug
When viewing pod logs with "Wrap lines" disabled, searching for a string causes the entire page content to shift left, partially hidden behind the sidebar navigation.
The original fix #16193, using css only, introduced another bug on the Pods List, so it will be reverted with #16218
Root Cause
Upstream PatternFly bug (react-log-viewer#106). PF LogViewer calls
scrollIntoView({ inline: 'center' })on search matches, which per the CSSOM View spec scrolls all scrollable ancestors. PF Drawer setsoverflow: hiddenon.pf-v6-c-drawer__main, which still acts as a scroll container for programmatic scrolling, causing the entire page to shift.Fix
A css-only fix is not available, so this adds a scroll event listener in
ResourceLogthat resetsscrollLeftto 0 on.pf-v6-c-drawer__main, preventing the page shift. CSS-only approaches were investigated but ruled out.Assisted by Claude code
After

Summary by CodeRabbit