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

Updating FFL fragments #72

Merged
merged 6 commits into from
Feb 26, 2024
Merged

Updating FFL fragments #72

merged 6 commits into from
Feb 26, 2024

Conversation

sree-cfa
Copy link
Contributor

@sree-cfa sree-cfa commented Feb 23, 2024

✍️ Description

Updated the checkbox and radio fieldset fragments to be able to take a collection of options.

The options param are values from classes that implement InputOption. They must have a value and label/helpText are optional. This is to simplify rendering for static checkboxes and have a more centralized way of referencing these values in html rendering and PDF generation.
*Backwards compatible.

<th:block th:with="genderOptions=${T(org.ilgcc.app.utils.GenderOption).values()}">
  <th:block th:replace="~{fragments/inputs/checkboxFieldset ::
    checkboxFieldset(inputName='childGender',
      label=#{children-ccap-info.gender-question},
      options=${genderOptions})}"/>
</th:block>

@sree-cfa sree-cfa marked this pull request as ready for review February 24, 2024 00:24
@cy-by cy-by requested a review from guiburato February 26, 2024 19:30
<div class="form-card__content">
<th:block th:replace="~{'fragments/inputs/checkboxFieldset' ::
<th:block th:replace="~{fragments/inputs/checkboxFieldset ::
Copy link
Contributor

Choose a reason for hiding this comment

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

@sree-cfa, how many options can the user check on this screen? If only one is possible, it should be Radio.

@@ -59,22 +54,10 @@
</th:block>
</th:block>

<th:block th:replace="~{'fragments/inputs/checkboxFieldset' ::
<th:block th:replace="~{fragments/inputs/checkboxFieldset ::
Copy link
Contributor

Choose a reason for hiding this comment

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

Gender and race should be checked as only one option, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question - I'll double check with Carlie. The design shows checkboxes implying it can have multiple answers so I'll leave it like that for now

@enyia21 enyia21 merged commit 44cfd31 into main Feb 26, 2024
5 checks passed
@enyia21 enyia21 deleted the new-fragment branch February 26, 2024 21:06
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.

3 participants