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

DT-1219: Allow un-selecting auth domains on snapshot create #1753

Merged
merged 3 commits into from
Feb 11, 2025
Merged

Conversation

snf2ye
Copy link
Contributor

@snf2ye snf2ye commented Feb 7, 2025

https://broadworkbench.atlassian.net/browse/DT-1219


Before:
https://drive.google.com/file/d/1SLNW12OJjLjTDkf8dQMae2NUKb_qVbc-/view?usp=drive_link
After:
https://drive.google.com/file/d/1yzDz2hJyQWYRWGvi7jVZ2nsC1JZ67iVx/view?usp=drive_link

Included Changes

  1. Update the placeholder text to say "Select Authorization Domain" - This follows patterns used in other dropdowns around TDR
  2. Add a "None" option that unselects auth domains
image image

Copy link

cypress bot commented Feb 7, 2025

jade-data-repo-ui    Run #4063

Run Properties:  status check passed Passed #4063  •  git commit a6503f251a ℹ️: Merge 12d91a758554740c1670d20fa1233f19d9fb93ca into 9d6322306d5b88759d9e41cdc65f...
Project jade-data-repo-ui
Branch Review sh/DT-1219
Run status status check passed Passed #4063
Run duration 02m 52s
Commit git commit a6503f251a ℹ️: Merge 12d91a758554740c1670d20fa1233f19d9fb93ca into 9d6322306d5b88759d9e41cdc65f...
Committer Shelby Holden
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 17
View all changes introduced in this branch ↗︎

@@ -191,9 +191,10 @@ function FullViewSnapshotButton({
sx={{ height: '2.5rem' }}
disabled={userGroups ? userGroups.length <= 1 : true}
options={userGroups ? userGroups.map((group) => group.groupName) : []}
name="authorization-domain"
name="Select Authorization Domain"
Copy link
Contributor

Choose a reason for hiding this comment

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

Changing the name here will affect tests, since that generates the IDs by which we grab the component.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, thanks!
I'm now formatting the id with lodash's "kebab case", so hopefully that will at least generate a more meaningful id.

Copy link

sonarqubecloud bot commented Feb 7, 2025

@snf2ye snf2ye changed the title DT-1219: Add none option for Auth domain selection on snapshot create DT-1219: Allow un-selecting auth domains on snapshot create Feb 7, 2025
@snf2ye snf2ye marked this pull request as ready for review February 7, 2025 21:20
@snf2ye snf2ye requested a review from a team as a code owner February 7, 2025 21:20
@snf2ye snf2ye requested review from rushtong and fboulnois and removed request for a team February 7, 2025 21:20
Copy link
Contributor

@rushtong rushtong left a comment

Choose a reason for hiding this comment

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

👍🏽

Copy link
Contributor

@fboulnois fboulnois 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, I will rebase on your changes, thank you 👍

@snf2ye snf2ye merged commit 4c9ff08 into develop Feb 11, 2025
9 checks passed
@snf2ye snf2ye deleted the sh/DT-1219 branch February 11, 2025 17:23
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.

4 participants