-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
fix(ui): fix sticky headers in grideditable #93648
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(ui): fix sticky headers in grideditable #93648
Conversation
@@ -486,7 +487,7 @@ class GridEditable< | |||
height={height} | |||
ref={this.refGrid} | |||
> | |||
<GridHead>{this.renderGridHead()}</GridHead> | |||
<GridHead sticky={stickyHeader}>{this.renderGridHead()}</GridHead> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it seems like we pass sticky
in two places now - to GridHead
and to each GridHeadCell
:
sentry/static/app/components/gridEditable/index.tsx
Lines 353 to 357 in f3b8437
<GridHeadCell | |
data-test-id="grid-head-cell" | |
key={`${i}.${column.key}`} | |
isFirst={i === 0} | |
sticky={stickyHeader} |
Shouldn’t it be enough to pass it to GridHead
now ? Your PR description also mentions you’ve moved where the sticky style gets applied - but I don’t see it being moved, only being added a second time 🤔.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Woops forgot to remove that, thanks for the catch
### Changes Move where the sticky style gets applied to fix issues with sticky header not working with scrolling. Also add a story to cover both sticky headers and scrolling ### Before  ### After 
### Changes Move where the sticky style gets applied to fix issues with sticky header not working with scrolling. Also add a story to cover both sticky headers and scrolling ### Before  ### After 
Changes
Move where the sticky style gets applied to fix issues with sticky header not working with scrolling. Also add a story to cover both sticky headers and scrolling
Before
After