fix: Header row in View table disappears when scrolling up #3105
Conversation
|
🚀 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. |
📝 WalkthroughWalkthroughReorganized table/header layout: introduced a fixed-position scrolling container, moved header rendering into the content container, switched header positioning to sticky, adjusted offsets and z-index, and refined header DOM and styles for separate name/type elements and pointer action placement. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
![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
🧹 Nitpick comments (3)
src/dashboard/TableView.scss (1)
12-23: Consider using CSS custom properties for repeated offset values.The value
96pxappears in multiple places (top: 96pxon line 12,padding-top: 96pxon line 23, and in Views.react.js). While the coordination is correct, using a CSS custom property would make future adjustments easier and reduce the risk of inconsistencies.📋 Example refactor using CSS variables
Define the variable in a shared stylesheet (e.g., globals.scss):
:root { --header-offset: 96px; }Then use it here:
.headers { position: sticky; - top: 96px; + top: var(--header-offset); left: 0; right: 0; background: #66637A; height: 30px; white-space: nowrap; z-index: 10; } .content { position: relative; - padding-top: 96px; + padding-top: var(--header-offset); min-height: 100vh; }src/dashboard/Data/Views/Views.react.js (2)
336-336: Magic number 96 repeated across multiple locations.The value
96pxappears as an inline style in three places (lines 336, 359, 367) and also in TableView.scss. Consider extracting this to a constant or CSS variable for maintainability.📋 Example refactor using a constant
Define a constant at the top of the file:
const HEADER_OFFSET = 96;Then use it:
- <div style={{ paddingTop: 96 }}> + <div style={{ paddingTop: HEADER_OFFSET }}> <div style={{ width: this.state.tableWidth }}> <div className={tableStyles.headers} style={{ width: this.state.tableWidth, right: 'auto', position: 'sticky', - top: 96, + top: HEADER_OFFSET, left: 0, zIndex: 10, }}And for the empty state:
- <div className={tableStyles.empty} style={{ top: 96 }}> + <div className={tableStyles.empty} style={{ top: HEADER_OFFSET }}>Also applies to: 359-359, 367-367
361-374: Inline styles override CSS classes, potentially causing confusion.The headers div uses
className={tableStyles.headers}but then overrides several properties with inline styles (position: 'sticky',top: 96,left: 0,zIndex: 10). This can make it difficult to understand the final computed styles. Consider either:
- Moving these overrides to the SCSS file if they're always needed, or
- Documenting why these inline overrides are necessary for this specific view.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/components/DataBrowserHeaderBar/DataBrowserHeaderBar.scsssrc/dashboard/Data/Views/Views.react.jssrc/dashboard/TableView.react.jssrc/dashboard/TableView.scss
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Node 18
- GitHub Check: Docker linux/amd64
🔇 Additional comments (3)
src/components/DataBrowserHeaderBar/DataBrowserHeaderBar.scss (1)
15-15: LGTM: Header height now visible.The height change from 0 to 30px makes the sticky header bar visible, which aligns with the PR's goal of fixing header visibility during scroll. This coordinates with the layout adjustments across other files.
src/dashboard/TableView.scss (1)
11-18: Sticky header positioning looks correct.The sticky positioning with
top: 96px,left: 0, andz-index: 10properly keeps the header visible during scroll. The removal of the sidebar offset logic (previouslyleft: 300px) simplifies the layout.src/dashboard/TableView.react.js (1)
41-41: LGTM: Headers correctly moved inside content container.Moving header rendering inside the content container aligns well with the sticky positioning strategy implemented in TableView.scss. The headers are computed earlier (line 41) and rendered within a
styles.headerswrapper (line 59), which coordinates properly with the CSS changes.Note: When data is empty or undefined,
headerswill be null and line 59 will render an empty div. This is acceptable since the empty div won't cause visual issues, but you could optionally conditionally render it:{headers && <div className={styles.headers}>{headers}</div>}Also applies to: 58-61
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/dashboard/Data/Views/Views.react.js (1)
359-384: Consider extracting layout constants for maintainability.The fixed-position scrolling container approach correctly addresses the sticky header requirement. However, the magic numbers
96(toolbar height) and300(sidebar width) are hardcoded in multiple places. Consider extracting these to shared constants or CSS variables to improve maintainability and prevent drift if these dimensions change.💡 Example refactor using constants
+const LAYOUT = { + TOOLBAR_HEIGHT: 96, + SIDEBAR_WIDTH: 300, +}; + renderContent() { // ... code ... return ( <div> <LoaderContainer loading={loading} solid={false}> <div style={{ position: 'fixed', - top: 96, - left: 300, + top: LAYOUT.TOOLBAR_HEIGHT, + left: LAYOUT.SIDEBAR_WIDTH, right: 0, bottom: 0, overflow: 'auto' }}>
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/dashboard/Data/Views/Views.react.jssrc/dashboard/Data/Views/Views.scss
🧰 Additional context used
🧬 Code graph analysis (1)
src/dashboard/Data/Views/Views.react.js (1)
src/components/Icon/Icon.react.js (1)
Icon(12-29)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Docker linux/amd64
🔇 Additional comments (6)
src/dashboard/Data/Views/Views.scss (4)
27-37: LGTM!The headerName styling is well-structured with proper text truncation and layout properties. The float-based layout with margin creates appropriate spacing for the split header design.
39-46: LGTM!The headerType styling appropriately creates a visual hierarchy with the smaller font size and muted color, distinguishing it from the primary headerName.
48-87: LGTM!The pointerIcon button styling is well-implemented with proper button resets, absolute positioning that works with the conditional padding in headerWrap, and good accessibility features including focus states and outline.
22-24: Consider browser compatibility for:has()pseudo-class if older browser support is required.The
:has()pseudo-class has limited support: Chrome/Edge 105+, Firefox 121+, Safari 15.4+. Since the project does not document minimum browser version requirements, verify whether older browser support is needed. If it is, implement a fallback using@supportsfeature detection or JavaScript-based conditional padding.src/dashboard/Data/Views/Views.react.js (2)
369-381: LGTM!The sticky header positioning is correctly implemented within the fixed scrolling container. The inline styles appropriately override the CSS class to achieve the sticky behavior, and the background color matches the headerWrap styling in the SCSS file.
606-641: LGTM!The refactored header rendering is well-structured with excellent accessibility:
- Clear separation of name and type information
- Proper button semantics with
type="button"- Descriptive
aria-labelandtitleattributes- Focus management with
blur()after action- Event handling prevents unwanted propagation
The conditional rendering of the pointer button only for Pointer columns is appropriate.
# [8.2.0-alpha.18](8.2.0-alpha.17...8.2.0-alpha.18) (2026-01-07) ### Bug Fixes * Header row in View table disappears when scrolling up ([#3105](#3105)) ([2923e86](2923e86))
|
🎉 This change has been released in version 8.2.0-alpha.18 |
# [8.2.0](8.1.0...8.2.0) (2026-01-15) ### Bug Fixes * Data browser pagination is ignored when using browser navigation or page reload ([#3097](#3097)) ([bcc4d5f](bcc4d5f)) * Batch-navigation is active even if info panels are not visible ([#3053](#3053)) ([91b544a](91b544a)) * Calculated value drop-down in menu bar overlaps with info panel buttons ([#3116](#3116)) ([0f6f729](0f6f729)) * Context menu in data browser disappears behind menu bar ([#3106](#3106)) ([2c6c471](2c6c471)) * Data browser table headers misaligned when scrolling horizontally ([#3067](#3067)) ([f495dd1](f495dd1)) * Graph panel covers right-most columns of data browser table ([#3112](#3112)) ([00b0d70](00b0d70)) * Graph panel shows date tick labels on x-axis in local time instead of UTC ([#3111](#3111)) ([85d4946](85d4946)) * Header row in View table disappears when scrolling up ([#3105](#3105)) ([2923e86](2923e86)) * Info panel covers whole sidebar if fewer objects than panels in multi-panel scenario ([#3042](#3042)) ([dd3ba8d](dd3ba8d)) * Info panel not refreshing on script execution ([#3040](#3040)) ([f57e7e2](f57e7e2)) * Prefetch for multiple info panels in data browser doesn't refresh stale cached data ([#3080](#3080)) ([e71d4e6](e71d4e6)) * Right-click on info panel header selects the object ([#3082](#3082)) ([ae87114](ae87114)) * Saved filter is not recognized in data browser filter dialog ([#3108](#3108)) ([8a4ce15](8a4ce15)) * Some context menu sub-menu lists don't change background color on mouse hover in Safari browser ([#3109](#3109)) ([6269d18](6269d18)) ### Features * Add AI agent browser control for autonomous development ([#3114](#3114)) ([5940455](5940455)) * Add confirmation dialog to handle conflicts when migrating settings to server ([#3092](#3092)) ([ae50b8d](ae50b8d)) * Add getting related records to context menu of info panel header ([#3083](#3083)) ([2623802](2623802)) * Add graph panel for data visualization ([#3110](#3110)) ([1e15e27](1e15e27)) * Add keyboard shortcuts for quick actions in data browser ([#3073](#3073)) ([858d0cc](858d0cc)) * Add Node 24 support ([#3041](#3041)) ([8cf2735](8cf2735)) * Add storing data browser filters on server ([#3090](#3090)) ([b991734](b991734)) * Add storing data browser graphs to support multiple graphs per class ([#3113](#3113)) ([e76f605](e76f605)) * Add support for `Video` type in View table to display videos ([#3061](#3061)) ([bd4aa4f](bd4aa4f)) * Allow selecting objects by click-dragging over info panel headers ([#3074](#3074)) ([d6ef86c](d6ef86c)) * Auto-expand filter list when clicking on class in data browser ([#3101](#3101)) ([30a733c](30a733c)) * Execute scripts via right-click on info panel header column ([#3068](#3068)) ([2983741](2983741)) ### Performance Improvements * Add local caching for server-stored settings to reduce loading from server ([#3094](#3094)) ([409973a](409973a)) * Remove unnecessary data fetches from data browser pagination ([#3098](#3098)) ([bc59998](bc59998))
2 participants
New Pull Request Checklist
Summary by CodeRabbit
Bug Fixes
Style
✏️ Tip: You can customize this high-level summary in your review settings.