Skip to content

Conversation

@jpople
Copy link
Contributor

@jpople jpople commented Dec 3, 2025

Ticket ENG-1489

Description Of Changes

See description on #6813; some layouts have been tweaked from that PR and static column widths were added to fix the issues that came up in ENG-1748.

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 December 3, 2025 15:50
@jpople jpople requested review from speaker-ender and removed request for a team December 3, 2025 15:50
@vercel
Copy link

vercel bot commented Dec 3, 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 Dec 3, 2025 3:59pm
1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
fides-privacy-center Ignored Ignored Dec 3, 2025 3:59pm

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 3, 2025

Greptile Overview

Greptile Summary

This PR implements sticky column headers and action bars for multiple tables across the admin UI, improving usability when scrolling through long tables.

Key changes:

  • Added sticky: { offsetHeader: 40 } configuration to table props in custom fields, systems, and data discovery tables
  • Made action bars sticky using sticky -top-6 z-10 bg-white py-4 Tailwind classes
  • Added static column widths (100px, 160px, 200px) to specific columns to prevent layout issues
  • Enhanced LinkCell and ListExpandableCell components with containerProps parameter for better layout control using width constraints (max-w-96, min-w-36)
  • Disabled sticky PageHeaders (isSticky={false}) on pages with sticky table headers to prevent z-index stacking issues
  • Removed absolute positioning from AddSystemsMenu that conflicted with new sticky layout
  • Updated cypress test to remove scrollIntoView() and hover checks that are no longer needed with sticky headers
  • Changed systems page from FixedLayout to Layout with custom width calculation

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The changes are well-structured, focused on UI improvements with proper z-index management and layout adjustments. Static column widths prevent layout issues that arose in the previous iteration. The containerProps addition to cell components is cleanly implemented and backward compatible. Test adjustments appropriately reflect the UI changes.
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
clients/admin-ui/src/features/common/table/cells/LinkCell.tsx 5/5 Added Flex container wrapper with optional containerProps for layout control in table cells
clients/admin-ui/src/features/common/table/cells/ListExpandableCell.tsx 5/5 Added containerProps parameter to allow custom Flex styling for table cell layout
clients/admin-ui/src/features/data-discovery-and-detection/action-center/hooks/useDiscoveredAssetsTable.tsx 5/5 Added sticky header config and static widths (200px, 160px) to specific columns
clients/admin-ui/src/features/system/SystemsTable.tsx 5/5 Made action bar sticky with proper positioning and moved Modal outside sticky container
clients/admin-ui/src/features/system/table/useSystemsTable.tsx 5/5 Added sticky header config with offsetHeader 40px, containerProps for LinkCell and ListExpandableCell with width constraints
clients/admin-ui/src/pages/systems/index.tsx 5/5 Changed from FixedLayout to Layout, set PageHeader isSticky to false, and added custom width to main container

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.

18 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Copy link
Contributor

@speaker-ender speaker-ender left a comment

Choose a reason for hiding this comment

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

Approving with known issue of horizontal scroll bar misalignment.
Follow up will refactor to use a max-height on the table with overflow to avoid stacked sticky elements.

@jpople jpople added this pull request to the merge queue Dec 3, 2025
Merged via the queue into main with commit d6840b0 Dec 3, 2025
46 of 47 checks passed
@jpople jpople deleted the jpople/eng-1489/sticky-column-headers branch December 3, 2025 20:13
jjdaurora pushed a commit that referenced this pull request Dec 5, 2025
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