Conversation
|
wg-hmrc
left a comment
wg-hmrc
left a comment
There was a problem hiding this comment.
The branch name doesn’t match the PR title, and the commits reference a different ticket ID. Can you clarify whether this is intentional?
| linkTextUrlTwo = "monthlyreturns.selectSubcontractors.deselectAll.link", | ||
| linkUrlTwo = controllers.monthlyreturns.routes.SelectSubcontractorsController.onPageLoadNonEmpty(Some(false)).url | ||
| ) | ||
| @if(subcontractors.length > 0) { |
There was a problem hiding this comment.
| @if(subcontractors.length > 0) { | |
| @if(subcontractors.nonEmpty) { |
We can do this instead and avoid unnecessary count
| </div> | ||
| </div> | ||
| """ | ||
| rows = if (subcontractors == Nil) Seq(Seq(TableRow( |
There was a problem hiding this comment.
Use isEmpty or nonEmpty over == Nil for readability
| value="${subcontractor.id}" | ||
| ${if (form.value.toList.flatMap(_.subcontractorsToInclude).contains(subcontractor.id)) "checked" else ""} | ||
| > | ||
| <label class="govuk-label govuk-checkboxes__label" for="${subcontractor.name.split(',')(0).toLowerCase}"> |
There was a problem hiding this comment.
The <label for="..."> does not match the checkbox id. The for attribute should reference the input id to ensure correct accessibility behaviour.
| ${if (form.value.toList.flatMap(_.subcontractorsToInclude).contains(subcontractor.id)) "checked" else ""} | ||
| > | ||
| <label class="govuk-label govuk-checkboxes__label" for="${subcontractor.name.split(',')(0).toLowerCase}"> | ||
| <span class="govuk-visually-hidden">Include ${subcontractor.name}</span> |
There was a problem hiding this comment.
Include is hard-coded, please move it to the translation file
| TableRow( | ||
| content = HtmlContent( | ||
| s""" | ||
| <div class="govuk-checkboxes govuk-checkboxes--small" data-module="govuk-checkboxes"> |
There was a problem hiding this comment.
not a big fan of hardcoding html, this can cause accessibility and maintenance issues, did you try using the GovUK components ?
| <input | ||
| class="govuk-checkboxes__input" | ||
| id="${subcontractor.id}" | ||
| name="subcontractorsToInclude.$index" |
There was a problem hiding this comment.
| name="subcontractorsToInclude.$index" | |
| name="subcontractorsToInclude[$index]" |
3 participants
No description provided.