8000 [VAULT-34808] UI: move the `radio` block in `FormField` under the HDS block by didoo · Pull Request #30555 · hashicorp/vault · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[VAULT-34808] UI: move the radio block in FormField under the HDS block #30555

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

Merged
merged 3 commits into from
May 14, 2025

Conversation

didoo
Copy link
Collaborator
@didoo didoo commented May 8, 2025

Description

What does this PR do?

  • updated logic in isHdsFormField of FormField to include the editType === 'radio' use case
  • moved template logic for editType === 'radio' in FormField under the isHdsField block
    • because the HDS input doesn't have an onChange argument, I had to add a new setAndBroadcastRadio action in the controller

Jira ticket: https://hashicorp.atlassian.net/browse/VAULT-34808

TODO only if you're a HashiCorp employee

  • Backport Labels: If this fix needs to be backported, use the appropriate backport/ label that matches the desired release branch. Note that in the CE repo, the latest release branch will look like backport/x.x.x, but older release branches will be backport/ent/x.x.x+ent.
    • LTS: If this fixes a critical security vulnerability or severity 1 bug, it will also need to be backported to the current LTS versions of Vault. To ensure this, use all available enterprise labels.
  • ENT Breakage: If this PR either 1) removes a public function OR 2) changes the signature
    of a public function, even if that change is in a CE file, double check that
    applying the patch for this PR to the ENT repo and running tests doesn't
    break any tests. Sometimes ENT only tests rely on public functions in CE
    files.
  • Jira: If this change has an associated Jira, it's referenced either
    in the PR description, commit message, or branch name.
  • RFC: If this change has an associated RFC, please link it in the description.
  • ENT PR: If this change has an associated ENT PR, please link it in the
    description. Also, make sure the changelog is in this PR, not in your ENT PR.

@github-actions github-actions bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label May 8, 2025
@didoo didoo added this to the 1.20.0-rc milestone May 8, 2025
@didoo didoo force-pushed the VAULT-34671/formfield-migrate-radio_VAULT-34742 branch from 084e541 to 665a654 Compare May 8, 2025 10:40
Copy link
github-actions bot commented May 8, 2025

CI Results:
All Go tests succeeded! ✅

@didoo didoo force-pushed the VAULT-34671/formfield-migrate-radio_VAULT-34742 branch 3 times, most recently from 0dffe69 to d245fa1 Compare May 8, 2025 19:55
@didoo didoo changed the base branch from main to VAULT-34671/improve-form-field-test-coverage_VAULT-34873 May 8, 2025 19:55
@didoo didoo force-pushed the VAULT-34671/formfield-migrate-radio_VAULT-34742 branch 2 times, most recently from e406bb1 to 6664da0 Compare May 9, 2025 20:12
@didoo didoo force-pushed the VAULT-34671/improve-form-field-test-coverage_VAULT-34873 branch 5 times, most recently from 793c5d3 to 914d1a6 Compare May 12, 2025 21:00
@didoo didoo force-pushed the VAULT-34671/formfield-migrate-radio_VAULT-34742 branch 3 times, most recently from 174a9af to 810c1e6 Compare May 13, 2025 09:09
@didoo didoo force-pushed the 8000 VAULT-34671/improve-form-field-test-coverage_VAULT-34873 branch from 914d1a6 to 509a832 Compare May 13, 2025 09:17
@didoo didoo force-pushed the VAULT-34671/formfield-migrate-radio_VAULT-34742 branch from 810c1e6 to 5d3f5c3 Compare May 13, 2025 09:19
@didoo didoo force-pushed the VAULT-34671/improve-form-field-test-coverage_VAULT-34873 branch 2 times, most recently from 9aaea0b to d2a44e6 Compare May 13, 2025 10:15
@didoo didoo force-pushed the VAULT-34671/formfield-migrate-radio_VAULT-34742 branch from 5d3f5c3 to f3e4d6b Compare May 13, 2025 10:17
@didoo didoo force-pushed the VAULT-34671/improve-form-field-test-coverage_VAULT-34873 branch from d2a44e6 to bbf755a Compare May 13, 2025 10:36
@didoo didoo force-pushed the VAULT-34671/formfield-migrate-radio_VAULT-34742 branch from f3e4d6b to 5e71766 Compare May 13, 2025 10:37
@didoo didoo marked this pull request as ready for review May 13, 2025 11:46
@didoo didoo requested a review from a team as a code owner May 13, 2025 11:46
@didoo
Copy link
Collaborator Author
didoo commented May 13, 2025

@Monkeychip can you (or someone else) have a look at this other PR? It is built on top of #30345, so we have a "standard" for how to write the tests for the form fields migrated to HDS. Let me also know if you want me to take screenshots of before/after the changes for some of the pages.

Copy link

Build Results:
All builds succeeded! ✅

{{#if this.hasRadioSubText}}
<F.HelperText data-test-help-text={{val.subText}}>{{val.subText}}</F.HelperText>
{{else if this.hasRadioHelpText}}
<F.HelperText data-test-help-text={{val.helpText}}>{{val.helpText}}</F.HelperText>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In this case the @helpText instead of being hidden behind a tooltip, is exposed as field's helper text (better for accessibility). See thread for context: https://hashicorp.slack.com/archives/C05DGFEBCP2/p1746701234173239

@@ -205,6 +205,12 @@ export default class FormFieldComponent extends Component {
this.setAndBroadcast(updatedValue);
}
@action
setAndBroadcastRadio(item) {
// we want to read the original value instead of `event.target.value` so we have `false` (boolean) and not `"false"` (string)
Copy link
Contributor

Choose a reason for hiding this comment

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

thank you for the comment!

Monkeychip
Monkeychip previously approved these changes May 13, 2025
Copy link
Contributor
@Monkeychip Monkeychip left a comment

Choose a reason for hiding this comment

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

Great work!

Base automatically changed from VAULT-34671/improve-form-field-test-coverage_VAULT-34873 to main May 13, 2025 19:59
@didoo didoo dismissed Monkeychip’s stale review May 13, 2025 19:59

The base branch was changed.

@didoo didoo force-pushed the VAULT-34671/formfield-migrate-radio_VAULT-34742 branch 2 times, most recently from 2662e40 to 128ee85 Compare May 13, 2025 20:44
@didoo
Copy link
Collaborator Author
didoo commented May 13, 2025

@Monkeychip I had to rebase to fix a couple of conflicts, can you re-approve? thanks

@didoo didoo force-pushed the VAULT-34671/formfield-migrate-radio_VAULT-34742 branch from 128ee85 to f245ad6 Compare May 14, 2025 10:55
@didoo didoo force-pushed the VAULT-34671/formfield-migrate-radio_VAULT-34742 branch from f245ad6 to 95cad10 Compare May 14, 2025 15:05
@Monkeychip Monkeychip merged commit 71254a4 into main May 14, 2025
33 checks passed
@Monkeychip Monkeychip deleted the VAULT-34671/formfield-migrate-radio_VAULT-34742 branch May 14, 2025 15:45
drivera258 pushed a commit that referenced this pull request May 14, 2025
… block (#30555)

* [UI] moved template logic for `radio` editType in `FormField` under the `isHdsField` block (#34742)

* [UI] added integration tests for `FormField` with editType=‘radio’ (#34742)

* [UI] fix broken tests (#34742)
drivera258 pushed a commit that referenced this pull request May 14, 2025
… block (#30555)

* [UI] moved template logic for `radio` editType in `FormField` under the `isHdsField` block (#34742)

* [UI] added integration tests for `FormField` with editType=‘radio’ (#34742)

* [UI] fix broken tests (#34742)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed hds-service HDS Service pr/no-changelog ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0