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

Refactor to use common system table and column dropdown #1590

Merged
merged 8 commits into from
Nov 1, 2022

Conversation

allisonking
Copy link
Contributor

@allisonking allisonking commented Oct 28, 2022

Closes #879

Code Changes

  • Bring ColumnDropdown and SystemsCheckboxTable to this repo
    • Really this should be done via fidesui, but I'm trying to stay unblocked so copied and pasted for now. https://github.com/ethyca/fidesui/pull/27 puts those components in fidesui, but it's awaiting review and some discussion
    • In theory, we should be able to swap the imports after it goes into fidesui and things should Just Work
  • Replace old ColumnDropdown component and use the new one (used by DatasetFieldsTable and DatasetCollectionView). These should look and behave the same as before
  • Refactor ScanResultsForm to use the new SystemsCheckboxTable. This should also look mostly the same as before
  • Adds column selection to the scan results page
  • Made some styling changes that became more noticeable when the table was able to change widths (due to choosing fewer columns)

Steps to Confirm

  • Turn on the config wizard in flags.json
  • Unskip the config wizard tests and go through those, or, if you do have real AWS creds, try those out

Pre-Merge Checklist

  • All CI Pipelines Succeeded
  • Documentation Updated:
    • documentation complete, or draft/outline provided (tag docs-team to complete/review on this branch)
    • documentation issue created (tag docs-team to complete issue separately)
  • Issue Requirements are Met
  • Relevant Follow-Up Issues Created
  • Update CHANGELOG.md

Description Of Changes

Closely related to https://github.com/ethyca/fidesui/pull/27

Screen.Recording.2022-10-28.at.2.48.12.PM.mov

@allisonking allisonking force-pushed the aking/879/use-common-system-table branch 3 times, most recently from c6c644d to 36b4e92 Compare October 28, 2022 18:29
@allisonking allisonking force-pushed the aking/879/use-common-system-table branch from 36b4e92 to f768dfd Compare October 28, 2022 18:59
@allisonking
Copy link
Contributor Author

Failing cypress tests seem to be a problem on main as well 😕

@allisonking allisonking marked this pull request as ready for review October 28, 2022 19:07
Copy link
Contributor

@LKCSmith LKCSmith left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@allisonking allisonking merged commit 819ebf7 into main Nov 1, 2022
@allisonking allisonking deleted the aking/879/use-common-system-table branch November 1, 2022 02:20
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.

Create reusable Systems table component
2 participants