chore: 🤖 use hds keyvalue component #3027
Conversation
chore: 🤖 key value migration test: 💍 refactor tests fix: 🐛 vars, tests fix: 🐛 test fix: 🐛 include custom label fix: 🐛 empty key/value fix: 🐛 filter logic fix: 🐛 comment fix: 🐛 comments refactor: 💡 comments and helper method chore: 🤖 undo file change
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
ui/admin/tests/integration/components/form/field/key-value-test.js
Outdated
Show resolved
Hide resolved
ui/admin/app/components/form/credential-library/vault-ssh-certificate/index.hbs
Show resolved
Hide resolved
Co-authored-by: Cristiano Rastelli <[email protected]>
| disabled={{@disabled}} | ||
| > | ||
| <:header as |H|> | ||
| {{#if @legend}} |
There was a problem hiding this comment.
Should this be optional? It seems we should always have a legend to be accessible
There was a problem hiding this comment.
Do we have a use cases for not having a legend? I also lean towards this not being optional.
| {{/if}} | ||
| </R.Field> | ||
|
|
||
| {{#unless this.hideValueField}} |
There was a problem hiding this comment.
Shouldn't this be hiding the key field instead of the value field if we just want one field? Though should this even be necessary?
| </R.Field> | ||
| {{/unless}} | ||
|
|
||
| <R.DeleteRowButton |
There was a problem hiding this comment.
It looks like we need to hide this if there's no input yet according to HDS suggestions.
| <:row as |R|> | ||
| <R.Field as |F|> | ||
| <F.Label>{{or @keyLabel (t 'form.key.label')}}</F.Label> | ||
| {{#if (eq @keyFieldType 'select')}} |
There was a problem hiding this comment.
I'm wondering if this component is trying too hard to simplify the HDS component without adding enough convenience for the limitations it now builds in.
It seems the hideValueField is only necessary because we need to handle the option of one field or two fields but we actually should be able to handle any number of fields as HDS also supports generic fields and not just inputs which we now limit.
Would it make more sense to cut down and remove the baked in fields (as well as the switches for keyFieldType) and just let the consumer handle adding in the fields? We could just yield back the <:row as |R|> to the consumer to handle this. I also notice all the keys/values also share the same name/errors which doesn't seem right.
There was a problem hiding this comment.
Looking over this and Zed's comment, I think I would lean towards allowing the consumer to handle this by yielding back. That should give us the flexibility without this complexity I think.
4 participants
Description
https://hashicorp.atlassian.net/browse/ICU-17397
This PR migrates to use hds key value inputs component.
Note: This pr only migrates a couple of forms with
textandselectinputs. The remaining forms will be migrated once this pattern is established.Screenshots (if appropriate)
sample error :
No change in response
critical options with data
critical options without data
account_claims_map with data
account_claims_map without data
How to Test
Checklist
[ ] I have added steps to reproduce and test for bug fixes in the description[ ] I have addeda11y-testslabel to run a11y audit tests if neededPCI review checklist
Examples of changes to security controls include using new access control methods, adding or removing logging pipelines, etc.