Skip to content

Conversation

@gilluminate
Copy link
Contributor

@gilluminate gilluminate commented Jan 6, 2026

Ticket ENG-702

Description Of Changes

Migrated the consent reporting table from FidesTableV2 (TanStack Table with Chakra UI) to Ant Design Table component.

Replaced custom table components with Ant Design equivalents and created a reusable EllipsisCell component for text truncation with tooltips (for tables to using fixed width columns)

Code Changes

  • Replaced FidesTableV2 with Ant Design Table component
  • Replaced TanStack Table column definitions with Ant Design column format
  • Created reusable EllipsisCell component
  • Fixed Ant table styling bug for border radius on scrollable tables

Steps to Confirm

  1. Navigate to the Consent Reporting page
  2. Verify the table displays all consent records correctly
  3. Verify date/time displays as relative time (e.g., "1 day ago")
  4. Verify text truncation with ellipsis and tooltips on long values
  5. Verify TCF preference tags with info icon look correctly and open the modal.
  6. Verify email links are clickable
  7. Verify pagination works correctly
  8. Verify date range filtering works
  9. Verify download and lookup modals open correctly

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

@vercel
Copy link

vercel bot commented Jan 6, 2026

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

Project Deployment Review Updated (UTC)
fides-plus-nightly Ready Ready Preview, Comment Jan 9, 2026 6:59pm
1 Skipped Deployment
Project Deployment Review Updated (UTC)
fides-privacy-center Ignored Ignored Jan 9, 2026 6:59pm

@gilluminate gilluminate closed this Jan 6, 2026
@gilluminate gilluminate reopened this Jan 6, 2026
@gilluminate gilluminate force-pushed the gill/ENG-702/migrate-consent-reporting-table branch from c145a31 to dc4816e Compare January 6, 2026 22:55
@gilluminate gilluminate marked this pull request as ready for review January 6, 2026 23:06
@gilluminate gilluminate requested a review from a team as a code owner January 6, 2026 23:06
@gilluminate gilluminate requested review from lucanovera and removed request for a team January 6, 2026 23:06
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 6, 2026

Greptile Summary

This PR successfully migrates the consent reporting table from FidesTableV2 (TanStack Table with Chakra UI) to Ant Design Table component.

Key Changes:

  • Replaced TanStack Table column definitions with Ant Design ColumnsType format in the columns hook
  • Converted custom table cell components to inline render functions that use Ant Design components
  • Created reusable EllipsisCell component for text truncation with tooltips across fixed-width columns
  • Updated Cypress tests to work with Ant Table DOM structure (changed selectors from fidesTable-body to consent-reporting-table)
  • Fixed Ant table styling bug for border radius on horizontally scrollable tables
  • Removed unnecessary table wrapper div and simplified layout structure

Implementation Quality:

  • Good use of useCallback for onTcfDetailViewClick to prevent unnecessary re-renders
  • Proper memoization of columns with useMemo and correct dependency array
  • Clean migration that maintains all existing functionality (date filtering, pagination, TCF badges, modals)
  • Well-documented EllipsisCell component with clear JSDoc examples

Confidence Score: 5/5

  • This PR is safe to merge with no issues found
  • The migration is clean and well-executed with proper test updates. All Cypress tests were updated to match the new Ant Design Table DOM structure. The code follows React best practices with proper memoization and callbacks. The new EllipsisCell component is reusable and well-documented. No functional regressions or code quality issues detected.
  • No files require special attention

Important Files Changed

Filename Overview
clients/admin-ui/src/pages/consent/reporting/index.tsx Successfully migrated from FidesTableV2 to Ant Design Table, replaced TanStack Table with Ant columns, added useCallback optimization
clients/admin-ui/src/features/consent-reporting/hooks/useConsentReportingTableColumns.tsx Converted TanStack column definitions to Ant Design ColumnsType format, replaced cell renderers with inline render functions
clients/admin-ui/src/features/common/table/cells/EllipsisCell.tsx New reusable component for text truncation with tooltips using Ant Typography.Text ellipsis feature
clients/admin-ui/cypress/e2e/consent-reporting.cy.ts Updated test selectors and assertions to work with Ant Design Table DOM structure, changed row count expectations

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.

6 files reviewed, 5 comments

Edit Code Review Agent Settings | Greptile

@gilluminate
Copy link
Contributor Author

@greptileai

Copy link
Contributor

@jack-gale-ethyca jack-gale-ethyca left a comment

Choose a reason for hiding this comment

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

Looks great

Copy link
Contributor

@lucanovera lucanovera left a comment

Choose a reason for hiding this comment

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

The new table is looking good! Code changes look good. Just a few small comments for improvement but I'll leave the approve too:

  1. The "User device ID" and "Preference ID" have ellipsis but no tooltip to see the full text. The original table didn't have those either, but I think it's important to be able to see the full id.
  2. The relative timestamp column doesn't show ellipsis properly and wraps to a 2nd line:
Captura de pantalla 2026-01-09 a la(s) 11 13 19 a  m

@gilluminate
Copy link
Contributor Author

gilluminate commented Jan 9, 2026

  1. The "User device ID" and "Preference ID" have ellipsis but no tooltip to see the full text. The original table didn't have those either, but I think it's important to be able to see the full id.

@lucanovera Both the old and new tables use a title attribute for the built-in browser tooltip. This is the default behavior for Ant Table's ellipsis property. Maybe you aren't hovering long enough? (it takes a second to kick in)

@lucanovera
Copy link
Contributor

  1. The "User device ID" and "Preference ID" have ellipsis but no tooltip to see the full text. The original table didn't have those either, but I think it's important to be able to see the full id.

@lucanovera Both the old and new tables use a title attribute for the built-in browser tooltip. This is the default behavior for Ant Table's ellipsis property. Maybe you aren't hovering long enough? (it takes a second to kick in)

I do see the built-in browser tooltip. I meant we could add the Ant Tooltip that will let you copy the text too.

@gilluminate
Copy link
Contributor Author

@lucanovera fixed! I've added an EllipsisCell component and applied that to other tables that could benefit as well.

@gilluminate gilluminate enabled auto-merge January 9, 2026 19:05
@gilluminate gilluminate added this pull request to the merge queue Jan 9, 2026
Merged via the queue into main with commit 9954832 Jan 9, 2026
44 of 45 checks passed
@gilluminate gilluminate deleted the gill/ENG-702/migrate-consent-reporting-table branch January 9, 2026 19:18
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.

4 participants