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

Migrate vendor selector to Ant #5459

Merged
merged 5 commits into from
Nov 5, 2024

Conversation

gilluminate
Copy link
Contributor

@gilluminate gilluminate commented Nov 4, 2024

Closes HJ-142

Description Of Changes

Vendor selector is a bit complex, so we're migrating it separately from the other Select components first.

Code Changes

  • Create new AutosuggestSuffix as reusable typeahead helper
  • Migrate the VendorSelector component to use Ant's Select component

Steps to Confirm

  • visit Manage your vendors page http://localhost:3000/consent/configure
  • Click "Add Vendors" button
  • Click "Add custom vendor" button
  • Ensure vendor picker select works as expected, including: selecting from the list, using the tab key to complete the autosuggestion, adding a new value by pressing the tab key (or click away), adding a new value by clicking that option from the menu.

Pre-Merge Checklist

  • All CI Pipelines Succeeded
  • Issue Requirements are Met
  • Relevant Follow-Up Issues Created
  • Update CHANGELOG.md

* visit Manage your vendors page http://localhost:3000/consent/configure
* Click "Add Vendors" button
* Click "Add custom vendor" button
* Ensure vendor picker select works as expected, including: selecting from the list, using the tab key to complete the autosuggestion, adding a new value by pressing the tab key (or click away), adding a new value by clicking that option from the menu.
Copy link

vercel bot commented Nov 4, 2024

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

1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
fides-plus-nightly ⬜️ Ignored (Inspect) Visit Preview Nov 5, 2024 6:09pm

Copy link

cypress bot commented Nov 4, 2024

fides    Run #10797

Run Properties:  status check passed Passed #10797  •  git commit 3925cfbc7f ℹ️: Merge 4aa901cc4f619afca0acac10b1f5e1bfcfbf7d07 into ba4c825f21a3fc44507b071193e3...
Project fides
Branch Review refs/pull/5459/merge
Run status status check passed Passed #10797
Run duration 00m 38s
Commit git commit 3925cfbc7f ℹ️: Merge 4aa901cc4f619afca0acac10b1f5e1bfcfbf7d07 into ba4c825f21a3fc44507b071193e3...
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 4
View all changes introduced in this branch ↗︎

if (newValue) {
const newVendorId = options.some((opt) => opt.value === newValue.value)
? newValue.value
: undefined;
setFieldValue("vendor_id", newVendorId);
await setValue(
Copy link
Contributor

Choose a reason for hiding this comment

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

Didn't know you could explicitly await a state call, this is a much nicer solution to that error message bug.

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.

Working largely as expected for me-- can we get the status prop set on the Select so it shows up with the red error border when there's an error?

Screenshot 2024-11-05 at 10 48 50

@gilluminate
Copy link
Contributor Author

can we get the status prop set on the Select so it shows up with the red error border when there's an error?

@jpople updated! Thanks for catching that.

@gilluminate gilluminate merged commit ce5188d into main Nov 5, 2024
13 checks passed
@gilluminate gilluminate deleted the gill/HJ-142/migrate-vendor-selector-to-ant branch November 5, 2024 21:38
Copy link

cypress bot commented Nov 5, 2024

fides    Run #10807

Run Properties:  status check passed Passed #10807  •  git commit ce5188d384: Migrate vendor selector to Ant (#5459)
Project fides
Branch Review main
Run status status check passed Passed #10807
Run duration 00m 36s
Commit git commit ce5188d384: Migrate vendor selector to Ant (#5459)
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 4
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