-
Notifications
You must be signed in to change notification settings - Fork 73
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
Migrate one-off Select inputs to Ant #5475
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
fides Run #11022
Run Properties:
|
Project |
fides
|
Branch Review |
refs/pull/5475/merge
|
Run status |
Passed #11022
|
Run duration | 00m 37s |
Commit |
2996d569e2 ℹ️: Merge 8eca9aeb4c289fe143bb0157afbdd632b7133a2d into 1c8b2d74db8ba4d8f883dbb1c30e...
|
Committer | Jason Gill |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
0
|
Pending |
0
|
Skipped |
0
|
Passing |
4
|
Upgrade your plan to view test results. | |
View all changes introduced in this branch ↗︎ |
ca762f0
to
7c0457e
Compare
7c0457e
to
732cb14
Compare
* Visit http://localhost:3000/consent/privacy-notices * Click [Add a privacy notice] button * About 1/2 way down the screen, click [Add notice children +] button * Button turns into new Select * This Select should be searchable * Clicking an option should revert the Select back to a Button NOTE: className changes `.select-children__menu` -> `.ant-select-dropdown` `.select-children__option` -> `.ant-select-item`
* Add a new system by visiting http://localhost:3000/add-systems/manual * In the "System details" section, add a few tags to the "System tags" field. * In the "Data processing properties" section, enable "This system performs profiling" toggle which reveals a Select menu * In the "Data processing properties" section, enable "This system transfers data" toggle which reveals another Select menu *Ensure this form still works when the following options have values: "Legal basis for profiling," "Legal basis for transfer," "Responsibility"
* Visit Data map report at http://localhost:3000/reporting/datamap * click the export button * Select an option and click download to ensure the correct filetype downloads.
* visit the System Inventory page `/systems` * Add a new system and configure the Iterable integration using Iterable API Key found in 1Password * Once added, a new "Consent Automation" accordion button will appear. Click to expand it * Make some selections from the list of available Privacy Notices and save * Ensure the form saves as expected
* Configure a Datastore connection (like BigQuery) by going to http://localhost:3000/datastore-connection and then clicking the kebab menu of the connection and clicking "configure" * Switch to the Dataset configuration tab * ensure the dataset select works as expected
* visit Connection manager http://localhost:3000/datastore-connection * Filter by Connection type, System type, Testing status, and Status. * Ensure each filter behaves as expected * Use the included (x) clear button to remove any filters
* Visit System Inventory http://localhost:3000/systems * Click an available system * Switch to Integrations tab * Choose "Link Integration" option * Select an integration to link * In the resulting form, choose datasets from the new Select
* Visit Messaging http://localhost:3000/messaging * Click "Add Message +" button * In the modal, make a selection with the new Select * Ensure expected behavior
* Visit Manage privacy notices http://localhost:3000/consent/privacy-notices * Click to edit a privacy notice that has more than one location * Notice the locations are displayed as pure Tags instead of a disabled Select with the tags represented. (This was originally done because Chakra didn't have any tags and we were piggy backing off the old Select component to solve it)
* Visit Manage privacy notices http://localhost:3000/consent/privacy-notices * Click "Add a privacy notice" * At the bottom of the screen click "+ Add a language" button * Select a language from the Ant select
* Visit Manage datasets (http://localhost:3000/dataset) * Click rows at least 3 levels deep until you can give rows categories in the Data Categories column * Click the [+] button to switch over to the Taxonomy Select * Ensure behavior is as expected
732cb14
to
c6df123
Compare
c6df123
to
3640d6a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested locally. Almost all working as intended:
- "Select notice children" on privacy notices
- Datamap report "export" file type
- Consent automation notice selects
- Datastore connection "dataset config"
- Connection manager filters
- System integrations dataset select
- Messaging type
- Privacy notice locations
- Privacy notice language picker
- Taxonomy select
I'm running into some bugs around the DictSuggestionSelects in the system form (tags, legal basis for profiling, legal basis for transfers, responsibility).
On a clean form, they're populated with a blank tag instead of empty:
And are throwing errors when I try to change the value:
Screen.Recording.2024-11-12.at.17.12.42.mov
Also, for "tags", on main we have this "Create [tag name]..." to indicate that users are creating new tags, is it feasible to include something like that on Ant? With just a blank select menu I think it's a little unclear how to use the field.
@jpople I think my latest commit solves those issues if you can pull latest. I'll investigate the "creating" question... |
@jpople it doesn't look like that option exists, but I created a feature request to see if they'll implement it for us ant-design/ant-design#51600 |
@jpople ok, I actually figured out how to get it working with the current options. Not as elegant as a new prop, but it works :) |
option.value === searchValue && | ||
!field.value.includes(searchValue) | ||
) { | ||
return `Create "${searchValue}"`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice workaround here!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the changes-- looking good!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there will be some changes for E2E tests.
fides Run #11029
Run Properties:
|
Project |
fides
|
Branch Review |
main
|
Run status |
Passed #11029
|
Run duration | 00m 39s |
Commit |
523c1ab716: Migrate one-off Select inputs to Ant (#5475)
|
Committer | Jason Gill |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
0
|
Pending |
0
|
Skipped |
0
|
Passing |
4
|
Upgrade your plan to view test results. | |
View all changes introduced in this branch ↗︎ |
Closes HJ-71
Description Of Changes
Migrating all of the "one-off" Select components to use the Ant Select component. There is more work to be done on the "custom" Select components, but that will come in another PR.
Code Changes
NOTE: className changes
.select-*__menu
->.ant-select-dropdown
.select-*__option
->.ant-select-item
Steps to Confirm
Pre-Merge Checklist
CHANGELOG.md
updatedmain
downgrade()
migration is correct and works