-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Use dynamic z indexes + use outline #6014
base: main
Are you sure you want to change the base?
Conversation
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.
PR Summary
- Remove unused z-indexes
- Implement dynamic z-indexes based on scroll state
- Switch from border to outline for styling
- Update class names for table cell selection state
- Improve visual consistency and performance of table components
7 file(s) reviewed, no comment(s)
981ea3a
to
89c1a6e
Compare
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.
PR Summary
(updates since last review)
- Remove unused z-indexes
- Introduce dynamic z-indexes based on scroll state
- Switch from border to outline for styling
- Improve visual hierarchy and interaction feedback
- Enhance table rendering performance and consistency
7 file(s) reviewed, no comment(s)
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.
PR Summary
(updates since last review)
- Remove unused z-indexes
- Introduce dynamic z-indexes based on scroll state
- Replace border usage with outline
- Add
isScrolledLeftSelector
andisScrolledTopSelector
for scroll state detection - Improve visual handling of table headers and cells when scrolled
4 file(s) reviewed, no comment(s)
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.
PR Summary
(updates since last review)
- Remove unused z-indexes
- Implement dynamic z-indexes based on scroll state
- Replace borders with outlines for styling
- Simplify
RecordTable
component by removingisScrolledTop
prop and related logic - Ensure no visual regressions due to styling changes
2 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
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.
PR Summary
(updates since last review)
- Remove unused z-indexes
- Implement dynamic z-indexes based on scroll state
- Replace borders with outlines
- Simplify
RecordTable
component logic - Adjust z-index of first three columns dynamically in
DraggableTableBody
2 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
Tested locally and I have a few issues on the table view 🤔 Looks like the hover is not triggered in the first cell for some reason |
Fix #5658
Enregistrement.de.l.ecran.2024-06-25.a.14.07.40.mov