fix: Keep vertical scrollbar visible in data browser with wide tables#3356
fix: Keep vertical scrollbar visible in data browser with wide tables#3356PreviousGod wants to merge 3 commits into
Conversation
Closes parse-community#2045 The vertical scrollbar was positioned at the right edge of the scrollable content area. When a table had many columns, users had to scroll all the way to the right to access the vertical scrollbar. This change separates horizontal and vertical scrolling: - .browser now only scrolls vertically (overflow-y: auto, overflow-x: hidden) - A new .browserContent wrapper handles horizontal scrolling - The sticky header and table rows scroll together horizontally - The vertical scrollbar is always visible on the viewport edge
|
I will reformat the title to use the proper commit message syntax. |
parse-github-assistant
Bot
changed the title
|
🚀 Thanks for opening this pull request! We appreciate your effort in improving the project. Please let us know once your pull request is ready for review. Tip
Note Please respond to review comments from AI agents just like you would to comments from a human reviewer. Let the reviewer resolve their own comments, unless they have reviewed and accepted your commit, or agreed with your explanation for why the feedback was incorrect. Caution Pull requests must be written using an AI agent with human supervision. Pull requests written entirely by a human will likely be rejected, because of lower code quality, higher review effort and the higher risk of introducing bugs. Please note that AI review comments on this pull request alone do not satisfy this requirement. Our CI and AI review are safeguards, not development tools. If many issues are flagged, rethink your development approach. Invest more effort in planning and design rather than using review cycles to fix low-quality code. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR separates vertical and horizontal scrolling in the data browser: ChangesBrowser Scrollbar Visibility
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 6 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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. 🔧 OpenGrep (1.20.0)OpenGrep fatal error (exit code 2): [00.14][ERROR]: Error: exception Unix_error: No such file or directory stat src/dashboard/Data/Browser/BrowserTable.react.js Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/dashboard/Data/Browser/BrowserTable.react.js`:
- Around line 607-649: The scroll reset is broken because
componentWillReceiveProps still sets this.tableRef.current.scrollTop but the
scrollable element is now the .browser container; add a new ref in the
constructor (this.browserRef = React.createRef()), attach it in render to the
outer .browser div (ref={this.browserRef}), and update componentWillReceiveProps
to set this.browserRef.current.scrollTop = 0 (replace uses of
this.tableRef.current.scrollTop) so filters/ordering/class changes correctly
reset scrolling; keep other logic unchanged.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a256152b-f04a-45b6-8c29-ec1179d8afe8
📒 Files selected for processing (2)
src/dashboard/Data/Browser/Browser.scsssrc/dashboard/Data/Browser/BrowserTable.react.js
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/dashboard/Data/Browser/BrowserTable.react.js`:
- Around line 46-56: The review points out accesses to
this.browserRef.current.scrollTop without guarding for a null ref; update the
BrowserTable component so every place that reads or writes
this.browserRef.current.scrollTop first checks that this.browserRef &&
this.browserRef.current (or equivalent) is truthy before using scrollTop
(including the branches that call this.setState(..., () => {
this.browserRef.current.scrollTop = 0; }) and the direct assignments like
this.browserRef.current.scrollTop = 0); ensure all occurrences in the component
are updated to safely skip or noop when the ref is null to avoid runtime errors
in SSR/tests/unmount races.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8d780b22-e61c-47dc-89f9-300bcdaeccb4
📒 Files selected for processing (1)
src/dashboard/Data/Browser/BrowserTable.react.js
1 participant
Closing in favor of #3357 which is a cleaner approach with fewer changes.