Skip to content
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: fixing broken corners of table with sticky header bottom when scrolled #3291

Merged
merged 3 commits into from
Feb 13, 2025

Conversation

dpitcock
Copy link
Member

@dpitcock dpitcock commented Feb 12, 2025

Fix for Sticky Header breaking bottom border at corners

The current implementation of the sticky header component can lead to a visual issue where the bottom corners of the sticky header jut out past the rounded borders of the table, causing a break in the visible border.

This pull request addresses this issue by making an enhancement to the useStickyHeader hook used by the Sticky Header and its parent component.

The key changes are:

Adding a new property called isStuckAtBottom to the object returned by the useStickyHeader hook. The isStuckAtBottom state is introduced to track whether the sticky header is aligned with the bottom edge of the table content. This is determined by comparing the bottom positions of the table container and the sticky header, and setting isStuckAtBottom accordingly. The internal table component then uses this state to apply an additional CSS class when the sticky header is in use, is stuck, and is stuck at the bottom edge, enabling the fix for the visual issue with the bottom border corners.

This fix should resolve the issue described in AWSUI-60364, providing a seamless visual experience for users when working with sticky headers in tables.

How has this been tested?

  • unit tests added and updated
  • integ tests uneffected
  • visual regression tests have only one expected change with the corrected borders in dev-v3-denpitco pipeline
Review checklist

The following items are to be evaluated by the author(s) and the reviewer(s).

Correctness

  • Changes include appropriate documentation updates.
  • Changes are backward-compatible if not indicated, see CONTRIBUTING.md.
  • Changes do not include unsupported browser features, see CONTRIBUTING.md.
  • Changes were manually tested for accessibility, see accessibility guidelines.

Security

Testing

  • Changes are covered with new/existing unit tests?
  • Changes are covered with new/existing integration tests?

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Copy link

codecov bot commented Feb 12, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.42%. Comparing base (f8f32c5) to head (a00ce27).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff            @@
##             main    #3291    +/-   ##
========================================
  Coverage   96.41%   96.42%            
========================================
  Files         791      791            
  Lines       22564    22572     +8     
  Branches     7727     7383   -344     
========================================
+ Hits        21756    21764     +8     
- Misses        754      801    +47     
+ Partials       54        7    -47     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dpitcock dpitcock marked this pull request as ready for review February 12, 2025 23:41
@dpitcock dpitcock requested a review from a team as a code owner February 12, 2025 23:41
@dpitcock dpitcock requested review from pan-kot and removed request for a team February 12, 2025 23:41
@dpitcock dpitcock added this pull request to the merge queue Feb 13, 2025
Merged via the queue into main with commit 187f672 Feb 13, 2025
39 checks passed
@dpitcock dpitcock deleted the denpitco-table-sticky-header-bottom-fix branch February 13, 2025 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants