Skip to content

Conversation

gilluminate
Copy link
Contributor

@gilluminate gilluminate commented Jul 18, 2025

Closes ENG-893

Description Of Changes

Removed hard-coded layout styles (margins) from status indicator components and moved spacing responsibility to their wrapper components. Replaced Chakra UI layout components (Box, Flex) with Ant Design Space components as part of the ongoing UI library migration.

Code Changes

  • Removed mr, mb margin props from all status indicator components
  • Replaced Box and Flex layout components with AntSpace as Space
  • Replaced createIcon with pure SVG components

Steps to Confirm

  1. Verify that status indicators in the data discovery tables maintain proper spacing between icon and text
  2. Check that the activity legend tooltip displays correctly with proper icon-text alignment
  3. Confirm that Action Center System aggregate status cells render without layout issues
CleanShot 2025-07-18 at 16 44 26@2x CleanShot 2025-07-18 at 16 42 15@2x

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
  • 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

@gilluminate gilluminate requested a review from jpople July 18, 2025 22:37
Copy link

vercel bot commented Jul 18, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
fides-plus-nightly ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 22, 2025 3:40pm
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
fides-privacy-center ⬜️ Ignored (Inspect) Jul 22, 2025 3:40pm

Copy link
Contributor

@jpople jpople left a comment

Choose a reason for hiding this comment

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

The circle-only and addition icons look good, but I think the removal arrow and classification tags ought to be shifted up a couple of pixels; the visual weight on these looks just a bit off and it feels off-center.

Screenshot 2025-07-21 at 11 54 51 Screenshot 2025-07-21 at 11 55 39

@gilluminate
Copy link
Contributor Author

The circle-only and addition icons look good, but I think the removal arrow and classification tags ought to be shifted up a couple of pixels; the visual weight on these looks just a bit off and it feels off-center.

@jpople Yeah, I left the mb on the one circle for the same reason. Ideally I think that fix should happen in the SVG and if I can't fix it there I'll tweak the mb on those also.

Copy link
Contributor

@jpople jpople left a comment

Choose a reason for hiding this comment

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

Looking good! Nice cleanup.

@gilluminate gilluminate force-pushed the gill/ENG-893/remove-all-layout-styles branch from bafe231 to 8549254 Compare July 22, 2025 15:36
@gilluminate gilluminate merged commit c10faa6 into main Jul 22, 2025
20 checks passed
@gilluminate gilluminate deleted the gill/ENG-893/remove-all-layout-styles branch July 22, 2025 15:53
Copy link

cypress bot commented Jul 22, 2025

fides    Run #13145

Run Properties:  status check passed Passed #13145  •  git commit c10faa6917: Remove all layout styles (margins) from StatusIndicators (#6360)
Project fides
Branch Review main
Run status status check passed Passed #13145
Run duration 00m 58s
Commit git commit c10faa6917: Remove all layout styles (margins) from StatusIndicators (#6360)
Committer Jason Gill
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 5
⚠️ You've recorded test results over your free plan limit.
Upgrade your plan to view test results.
View all changes introduced in this branch ↗︎

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