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

Classify data category dropdown #1110

Merged
merged 17 commits into from
Oct 3, 2022

Conversation

allisonking
Copy link
Contributor

@allisonking allisonking commented Sep 22, 2022

Visual part of #1077 (not integrated yet)

Code Changes

  • Refactor the existing DataCategoryInput to make it more reusable
  • Create ClassifiedDataCategoryInput which is two select boxes combined
  • Cypress component tests

Steps to Confirm

  • Since this doesn't exist in the app yet, the only way to see this is through running cypress component tests. There is one test called playground which will let you just play with the component

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

I thought I'd put up this PR up before attempting to integrate it into the app to make sure the behavior looks right. Given that https://github.com/ethyca/fidesctl-plus/issues/99 isn't in yet, can't integrate fully, but could perhaps get a little farther.

Screen.Recording.2022-09-22.at.2.38.08.PM.mov

Base automatically changed from aking-cypress-component-testing to main September 22, 2022 18:04
@allisonking allisonking force-pushed the aking-1077-classify-data-category-dropdown branch from 85b3c71 to 36f532a Compare September 22, 2022 18:16
@allisonking allisonking marked this pull request as ready for review September 22, 2022 19:35
@allisonking allisonking requested review from a team and ssangervasi September 22, 2022 19:35
@allisonking
Copy link
Contributor Author

@ssangervasi after your question about the design, I decided to just integrate the component into the page (with a TODO for what the mostLikelyCategories actually are—currently hard coded to the first 5 categories). That turned out to be a little more involved than I thought it would be due to circular dependencies, so I did some refactoring. Then, the cypress component test for DataCategoryInput didn't work anymore since I added a call to redux, so I also configured cypress component testing to have access to our store.

So that's a few extra commits, and would love if you could take one more look!

@allisonking
Copy link
Contributor Author

Updated:

Screen.Recording.2022-09-27.at.4.02.45.PM.mov

Copy link
Contributor

@ssangervasi ssangervasi left a comment

Choose a reason for hiding this comment

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

👍

* main:
  Restrict startsWith comparisons in CheckboxTree more (#1126)
  [1076] ui/plus: Approve dataset classification button (#1129)
  Do not rely on order for checking intercept results (#1131)
  Prepare 1.9.1 release (#1137)
  Bump fideslang to 1.3.1 (#1136)
  Prepare changelog for 1.9.0 release (#1134)
  Update CHANGELOG.md (#1132)
  Fix bug causing missing datamap rows (#1124)
  cls migration (#1060)
@ssangervasi ssangervasi force-pushed the aking-1077-classify-data-category-dropdown branch from 1d180d7 to f6ab5ff Compare October 1, 2022 01:49
@ssangervasi ssangervasi requested a review from a team October 3, 2022 17:21
@ssangervasi ssangervasi merged commit ac92cb6 into main Oct 3, 2022
@ssangervasi ssangervasi deleted the aking-1077-classify-data-category-dropdown branch October 3, 2022 17:56
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