[VAULT-34792] UI: refactor FormField template code to move checkboxList under the newly added HDS block

[didoo]

Description

What does this PR do?

• moved template logic for checkboxList of FilterInput under the isHdsField block
• added handling of label/helpText/subText/docLink + validation errors message for checkboxList
• moved validationWarning block out of the conditional so it can serve both HDS and non-HDS use cases

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

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]

CI Results:
All Go tests succeeded! ✅

[github-actions]

Build Results:
All builds succeeded! ✅

[hellobontempo]

This getter doesn't account for checking if there's helpText, I wonder if we should update that there so this div doesn't render unless helpText exists (instead of having to add it to every conditional wrapping helper text)

[didoo]

@hellobontempo what do you prefer? I can update the getter in this PR, or I can create a separate PR for this (no biggie, I just want to follow your existing processes).

[hellobontempo]

Looking at the code again, it looks like there is some additional cleanup worth tackling all at once. (Since FormFieldLabel relies on the boolean to pass along the helpText or not). If you want to tackle all of that now, a separate PR may be best. Though I think adding the second check here {{#if (and this.showHelpText @attr.options.helpText)}} might be smoothest for now and that tech cleanup can be postponed until the end. What do you think?

[didoo]

If you look at this PR #29960 you will see that the code is being duplicated here and here (and will be duplicated even more, the more we migrate components here; I am going to think how to abstract this "thing" once we have a few more instances, so for now we have to live with that duplication)

All to say that probably is better to do this at getter level, already (now). I am going to open a PR for this, as parallel stream of work.

[hellobontempo]

Great! I agree (just didn't want to disrupt your workflow 😄 )
Consolidating the getter to be something like this probably makes sense, and removing all references to @attr.options.helpText but I'll defer to you since you're in the thick of all that logic now.

  get showHelpText() {
    const { options } = this.args.attr;
    if (this.args.showHelpText !== false && options.helpText) return options.helpText;
    return '';
  }

[hellobontempo]

👏 one down!! 🎉

[didoo]

@Mongey @hellobontempo in one of my tests/spikes I added a comment here that says:

TODO! here I've replaced includes with eq because it was not working: includes expects a list/array as second argument, while here we're passing a single value - see: https://github.com/DockYard/ember-composable-helpers?tab=readme-ov-file#includes

and then changed the line to

checked={{eq option (get @model this.valuePath)}}

But I imagine is working correctly in production, so probably nothing to see here. But I wanted to raise it anyway, because I noticed the comment in one of my branches and I wanted to double check with you (that have a little bit more context/knowledge of the codebase :) )

[hellobontempo]

Thanks for calling this out. When I implemented this originally get @model this.valuePath is an array. So it sounds like we might need to make the input more flexible 🤔

[hellobontempo]

Actually, since multiple items can be selected I think it makes sense to keep the model value type an array (and no need to make it flexible and accept different value types)

[hellobontempo]

Great work! 🚀

As I understand it, we'll leave the showHelpText conditional for now (which renders an extra <div> if there's no help text) and update the getter separately to check for attr.options.helpText in a parallel PR)