Skip to content

Conversation

@jpople
Copy link
Contributor

@jpople jpople commented Oct 21, 2025

Ticket 1489

Description Of Changes

Changes column headers and action bars on some tables (specifically those using the new useAntTable hooks that are the primary element on their page) to be sticky. Also makes page headers on applicable pages nonsticky for compatibility.

Steps to Confirm

On the following tables, verify that page headers and action bars stick when scrolling:

  • System inventory
  • Custom fields table
  • Action center system aggregate results
  • Action center asset table

Pre-Merge Checklist

  • Issue requirements met
  • All CI pipelines succeeded
  • CHANGELOG.md updated
    • Add a db-migration This indicates that a change includes a database migration label to the entry if your change includes a DB migration
    • Add a high-risk This issue suggests changes that have a high-probability of breaking existing code label to the entry if your change includes a high-risk change (i.e. potential for performance impact or unexpected regression) that should be flagged
    • Updates unreleased work already in Changelog, no new entry necessary
  • UX feedback:
    • All UX related changes have been reviewed by a designer
    • No UX review needed
  • Followup issues:
    • Followup issues created
    • No followup issues
  • Database migrations:
    • Ensure that your downrev is up to date with the latest revision on main
    • Ensure that your downgrade() migration is correct and works
      • If a downgrade migration is not possible for this change, please call this out in the PR description!
    • No migrations
  • Documentation:
    • Documentation complete, PR opened in fidesdocs
    • Documentation issue created in fidesdocs
    • If there are any new client scopes created as part of the pull request, remember to update public-facing documentation that references our scope registry
    • No documentation updates required

@jpople jpople requested a review from a team as a code owner October 21, 2025 23:54
@jpople jpople requested review from vcruces and removed request for a team October 21, 2025 23:54
@vercel
Copy link

vercel bot commented Oct 21, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
fides-plus-nightly Ready Ready Preview Comment Oct 22, 2025 4:50pm
1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
fides-privacy-center Ignored Ignored Oct 22, 2025 4:50pm

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile Overview

Summary

Implemented sticky column headers and action bars for full-page tables using Ant Design's sticky table configuration. Made page headers non-sticky on affected pages for compatibility with the new sticky table elements.

Key changes:

  • Added sticky: { offsetHeader: 40 } config to four table hooks to enable sticky headers
  • Made action bars sticky using Tailwind classes (sticky -top-6 z-10 bg-white py-4)
  • Disabled sticky page headers (isSticky={false}) on four pages
  • Switched from FixedLayout to Layout on systems and action center pages
  • Added containerProps to LinkCell and ListExpandableCell for width constraints

Issues found:

  • LinkCell.tsx: Missing Flex import and using div instead of Flex component (violates style guide)
  • Two page files: Using inline style attributes instead of Tailwind classes (violates style guide)

Confidence Score: 3/5

  • Safe to merge after fixing the missing Flex import in LinkCell.tsx, which will cause runtime errors
  • The syntax error in LinkCell.tsx (missing Flex import for the suggested refactor) needs to be fixed. The inline style violations are style preferences that don't affect functionality. The core sticky header implementation is sound and consistent across all tables.
  • Pay close attention to clients/admin-ui/src/features/common/table/cells/LinkCell.tsx - it has style guide violations that should be addressed

Important Files Changed

File Analysis

Filename Score Overview
clients/admin-ui/src/features/common/table/cells/LinkCell.tsx 2/5 Added containerProps support but introduced issues: missing Flex import and using div instead of Flex component
clients/admin-ui/src/pages/settings/custom-fields/index.tsx 4/5 Disabled sticky page header and adjusted padding, but uses inline style instead of Tailwind
clients/admin-ui/src/pages/systems/index.tsx 4/5 Switched layouts, disabled sticky header, adjusted button positioning, but uses inline style

Sequence Diagram

sequenceDiagram
    participant Page as Page Component
    participant PageHeader as PageHeader
    participant Table as Table Component
    participant Hook as useAntTable Hook
    participant AntTable as Ant Design Table
    
    Page->>PageHeader: Render with isSticky=false
    Note over PageHeader: Page header non-sticky<br/>for compatibility
    
    Page->>Table: Render table component
    Table->>Hook: Initialize with sticky config
    Note over Hook: sticky: {<br/>  offsetHeader: 40<br/>}
    
    Table->>Table: Render action bar
    Note over Table: Action bar with classes:<br/>sticky -top-6 z-10<br/>bg-white py-4
    
    Table->>AntTable: Pass tableProps with sticky config
    AntTable->>AntTable: Apply sticky headers
    Note over AntTable: Column headers stick<br/>40px below viewport top
    
    Note over Page,AntTable: User scrolls down
    PageHeader->>PageHeader: Scrolls off screen
    Table->>Table: Action bar sticks to top
    AntTable->>AntTable: Headers stick below action bar
Loading

14 files reviewed, 4 comments

Edit Code Review Agent Settings | Greptile

Copy link
Contributor

@vcruces vcruces left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks great! I just noticed that when scrolling through the System Inventory table, the top border of the table header disappears. But is that just a detail, or perhaps just a visual effect?

Screen.Recording.2025-10-22.at.11.28.23.mov

.scrollIntoView()
.should("be.visible");
cy.getByTestId("discovery-status-icon-alert").realHover();
cy.getByTestId("discovery-status-icon-alert").should("exist");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The scrollIntoView was causing issues here because the sticky headers break horizontal scrolling somehow. Just streamlined this test, @gilluminate and I thought it was fine to check that the icon exists without needing to check the tooltip showing on hover.

@jpople jpople added this pull request to the merge queue Oct 22, 2025
Merged via the queue into main with commit 580162f Oct 22, 2025
45 of 46 checks passed
@jpople jpople deleted the jpople/eng-1489/sticky-column-headers branch October 22, 2025 17:17
gilluminate added a commit that referenced this pull request Oct 23, 2025
@greptile-apps greptile-apps bot mentioned this pull request Oct 30, 2025
18 tasks
@jpople jpople mentioned this pull request Dec 3, 2025
18 tasks
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.

3 participants