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

PLATUI-2942: Upgrade library to govuk-frontend v5.3.0 #292

Merged
merged 6 commits into from
Apr 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,18 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
For compatibility information see `govukFrontendVersion` and `hmrcFrontendVersion` in
[LibDependencies](project/LibDependencies.scala)

## [9.6.0] - 2024-04-10

### Changed

- Release new version of play-frontend-hmrc that includes govuk-frontend 5.3.0
- Added ADR for [deferring the inclusion of the password field](./docs/maintainers/adr/0021-defer-inclusion-of-password-field.md) from govuk-frontend v5.3.0

### Compatible with

- [hmrc/hmrc-frontend v6.15.0](https://github.com/hmrc/hmrc-frontend/releases/tag/v6.15.0)
- [alphagov/govuk-frontend v5.3.0](https://github.com/alphagov/govuk-frontend/releases/tag/v5.3.0)

## [9.5.0] - 2024-04-3

### Changed
Expand Down
50 changes: 50 additions & 0 deletions docs/maintainers/adr/0021-defer-inclusion-of-password-field.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
# Defer Inclusion of Password Field from govuk-frontend v5.3.0

* Status: accepted
* Date: 2024-08-04

## Context and Problem Statement

With the release of govuk-frontend v5.3.0, which includes a password field, we are considering whether to integrate this feature into our frontend library. Given that authentication for most services is handled by a central service (e.g., GG or the upcoming OLFG), the need for a library component for password fields may not be widespread. There are also considerations around the engineering cost of creating and maintaining such a component.

## Decision Drivers

* Centralized authentication services reducing the need for a password field component
* Engineering cost of maintenance and development
* Potential future demand for this component

## Considered Options

* Immediately include the password field feature from govuk-frontend v5.3.0
* Defer the inclusion of the password field feature until there is a demonstrated need

## Decision Outcome

Chosen option: "Defer the inclusion of the password field feature until there is a demonstrated need", because it allows us to focus on components with a clear demand and reduces the maintenance burden. This decision is reversible, and we remain open to revisiting it should a significant need for this component arise from our services.

### Positive Consequences

* Keeps the library focused on widely used components
* Reduces unnecessary maintenance and development work
* Flexible approach that can adapt to future demands

### Negative Consequences

* Teams needing the password field immediately may have to implement their own solutions

## Pros and Cons of the Options

### Immediately include the password field feature

* Good, because it provides immediate feature completeness.
* Bad, because it likely adds an unused feature for most teams, increasing maintenance overhead.

### Defer the inclusion of the password field feature

* Good, because it aligns our resources with current needs and reduces overhead.
* Good, because it leaves room to adapt based on future demand.
* Bad, because teams with immediate needs must find alternative solutions.

## Links

* [govuk-frontend v5.3.0 Release Notes](https://github.com/alphagov/govuk-frontend/releases/tag/v5.3.0)
1 change: 1 addition & 0 deletions docs/maintainers/adr/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
* [ADR-0018](0018-help-teams-implement-new-date-guidance.md) - Help teams implement new date guidance
* [ADR-0019](0019-add-welsh-language-support-to-govuk-accessible-autocomplete.md) - Add Welsh language support to the GOVUK accessible autocomplete wrapper
* [ADR-0020](0020-reduce-support-for-internet-explorer.md) - Reduce support for Internet Explorer
* [ADR-0021](0021-defer-inclusion-of-password-field.md) - Defer Inclusion of Password Field from govuk-frontend v5.3.0

<!-- adrlogstop -->

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,12 @@ trait Aliases {
type CookieBanner = viewmodels.cookiebanner.CookieBanner
val CookieBanner = viewmodels.cookiebanner.CookieBanner

type Message = viewmodels.cookiebanner.Message
val Message = viewmodels.cookiebanner.Message

type Action = viewmodels.cookiebanner.Action
val Action = viewmodels.cookiebanner.Action

type PageLayout = viewmodels.pagelayout.PageLayout
val PageLayout = viewmodels.pagelayout.PageLayout

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,6 @@ trait Utils extends UtilsSupport {
.map(p => s"$p/$file")
.getOrElse(uk.gov.hmrc.hmrcfrontend.controllers.routes.Assets.at(s"govuk/$file").url)

private[views] def govukPluralisedI18nAttributes(
translationKey: String,
pluralForms: Option[Map[String, String]]
): immutable.Iterable[Html] =
pluralForms.getOrElse(Map.empty).map { case (k, v) =>
Html(s"""data-i18n.$translationKey.${HtmlFormat.escape(k)}="${HtmlFormat.escape(v)}" """)
}
}

object Utils extends Utils
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@ package uk.gov.hmrc.govukfrontend.views.viewmodels

import play.api.libs.functional.syntax._
import play.api.libs.json._
import uk.gov.hmrc.govukfrontend.views.viewmodels.content.Content

case class FormGroup(
classes: Option[String] = None,
attributes: Map[String, String] = Map.empty
attributes: Map[String, String] = Map.empty,
afterInput: Option[Content] = None
)

object FormGroup {
Expand All @@ -31,13 +33,15 @@ object FormGroup {
implicit def jsonReads: Reads[FormGroup] =
(
(__ \ "classes").readNullable[String] and
(__ \ "attributes").readWithDefault[Map[String, String]](Map.empty)
(__ \ "attributes").readWithDefault[Map[String, String]](Map.empty) and
(__ \ "afterInput").readNullable[Content]
)(FormGroup.apply _)

implicit def jsonWrites: OWrites[FormGroup] =
(
(__ \ "classes").writeNullable[String] and
(__ \ "attributes").write[Map[String, String]]
(__ \ "attributes").write[Map[String, String]] and
(__ \ "afterInput").writeNullable[Content]
)(unlift(FormGroup.unapply))

}
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ import uk.gov.hmrc.govukfrontend.views.viewmodels.label.Label
* @param prefix optional content to display immediately before the `input`
* @param suffix optional content to display immediately after the `input`
* @param disabled optional `disabled` attribute for the `input`
* @param autocapitalize optional `autocapitalize` attribute for the `input`
* @param inputWrapper additional CSS classes/attributes to apply to the input wrapper
*/
case class Input(
id: String = "",
Expand All @@ -62,7 +64,9 @@ case class Input(
spellcheck: Option[Boolean] = None,
prefix: Option[PrefixOrSuffix] = None,
suffix: Option[PrefixOrSuffix] = None,
disabled: Option[Boolean] = None
disabled: Option[Boolean] = None,
autocapitalize: Option[String] = None,
inputWrapper: InputWrapper = InputWrapper.empty
)

object Input {
Expand All @@ -88,7 +92,9 @@ object Input {
(__ \ "spellcheck").readNullable[Boolean] and
(__ \ "prefix").readNullable[PrefixOrSuffix] and
(__ \ "suffix").readNullable[PrefixOrSuffix] and
(__ \ "disabled").readNullable[Boolean]
(__ \ "disabled").readNullable[Boolean] and
(__ \ "autocapitalize").readNullable[String] and
(__ \ "inputWrapper").readWithDefault[InputWrapper](defaultObject.inputWrapper)
)(Input.apply _)

implicit def jsonWrites: OWrites[Input] =
Expand All @@ -110,7 +116,9 @@ object Input {
(__ \ "spellcheck").writeNullable[Boolean] and
(__ \ "prefix").writeNullable[PrefixOrSuffix] and
(__ \ "suffix").writeNullable[PrefixOrSuffix] and
(__ \ "disabled").writeNullable[Boolean]
(__ \ "disabled").writeNullable[Boolean] and
(__ \ "autocapitalize").writeNullable[String] and
(__ \ "inputWrapper").write[InputWrapper]
)(unlift(Input.unapply))

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
* Copyright 2024 HM Revenue & Customs
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package uk.gov.hmrc.govukfrontend.views.viewmodels.input

import play.api.libs.functional.syntax._
import play.api.libs.json._

case class InputWrapper(
classes: Option[String] = None,
attributes: Map[String, String] = Map.empty
)

object InputWrapper {

val empty: InputWrapper = InputWrapper()

implicit def jsonReads: Reads[InputWrapper] =
(
(__ \ "classes").readNullable[String] and
(__ \ "attributes").readWithDefault[Map[String, String]](Map.empty)
)(InputWrapper.apply _)

implicit def jsonWrites: OWrites[InputWrapper] =
(
(__ \ "classes").writeNullable[String] and
(__ \ "attributes").write[Map[String, String]]
)(unlift(InputWrapper.unapply))

}
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@

<div class="govuk-width-container">
@beforeContentBlock
<main class="govuk-main-wrapper @mainClasses" id="main-content" role="main"@mainLang.filter(_.nonEmpty).map {mainLang => lang="@mainLang"}>
<main class="govuk-main-wrapper @mainClasses" id="main-content" @mainLang.filter(_.nonEmpty).map {mainLang => lang="@mainLang"}>
@contentBlock
</main>
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -15,38 +15,36 @@
*@

@import uk.gov.hmrc.govukfrontend.views.html.components._
@import scala.collection.immutable.ListMap

@this(govukTextarea: GovukTextarea,
govukHint: GovukHint)


@(params: CharacterCount)
@import params._
@require(name.nonEmpty && id.nonEmpty, "name and id should be non-empty")

@attrs = {
@maxLength.filter(_ > 0).map {value => data-maxlength="@value"}
@threshold.filter(_ > 0).map {value => data-threshold="@value"}
@maxWords.filter(_ > 0).map {value => data-maxwords="@value"}

@govukPluralisedI18nAttributes("characters-under-limit", charactersUnderLimitText)
@charactersAtLimitText match {
case Some(v) => { data-i18n.characters-at-limit="@{v}" }
case None => {}
}
@govukPluralisedI18nAttributes("characters-over-limit", charactersOverLimitText)
@govukPluralisedI18nAttributes("words-under-limit", wordsUnderLimitText)
@wordsAtLimitText match {
case Some(v) => { data-i18n.words-at-limit="@{v}" }
case None => {}
}
@govukPluralisedI18nAttributes("words-over-limit", wordsOverLimitText)
@govukPluralisedI18nAttributes(translationKey: String, pluralForms: Option[Map[String, String]]) = @{
pluralForms.getOrElse(Map.empty).map { case (key, value) =>
s"data-i18n.$translationKey.$key" -> value
}
}

@if(maxLength.isEmpty && maxWords.isEmpty) {
@textareaDescriptionText match {
case Some(v) => { data-i18n.textarea-description.other="@{v}" }
case None => {}
}
}
@dataAttributes = @{
val maybeDataAttributes = ListMap(
Copy link
Contributor

Choose a reason for hiding this comment

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

do we still need to use a ListMap here? The attrs as string OR map[string, string] is really confusing to work around - looks ok, feels like it's going to be worth seeing if we can do a better comparison when testing against output rendered by govuk stuff, can imagine more an more excluded / flakey tests 😵

Copy link
Contributor

Choose a reason for hiding this comment

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

IIRC ListMap fixed some failing unit tests, and yes - it's awful 😅

"data-module" -> "govuk-character-count",
"data-maxlength" -> maxLength.filter(_ > 0).map(_.toString).getOrElse(""),
"data-threshold" -> threshold.filter(_ > 0).map(_.toString).getOrElse(""),
"data-maxwords" -> maxWords.filter(_ > 0).map(_.toString).getOrElse(""),
) ++ govukPluralisedI18nAttributes("characters-under-limit", charactersUnderLimitText) ++ Map(
"data-i18n.characters-at-limit" -> charactersAtLimitText.getOrElse("")
) ++ govukPluralisedI18nAttributes("characters-over-limit", charactersOverLimitText) ++ govukPluralisedI18nAttributes("words-under-limit", wordsUnderLimitText) ++ Map(
"data-i18n.words-at-limit" -> wordsAtLimitText.getOrElse("")
) ++ govukPluralisedI18nAttributes("words-over-limit", wordsOverLimitText) ++ Map(
"data-i18n.textarea-description.other" -> textareaDescriptionText.filter(_ => maxLength.isEmpty && maxWords.isEmpty).getOrElse("")
)
maybeDataAttributes.filter { case (_, value) => value.nonEmpty }
}

@customTextArea = @{
Expand All @@ -61,8 +59,19 @@
}
}

<div class="govuk-character-count" data-module="govuk-character-count"@attrs>
@govukTextarea(Textarea(
@characterCountMessage = {
@govukHint(Hint(id=Some(id+"-info"),
classes = toClasses("govuk-character-count__message", if (countMessageClasses.isEmpty) "" else s" $countMessageClasses"),
content = customTextArea
))
}

@characterCountMessagePlusOptionalAfterInput = {
@characterCountMessage
@formGroup.afterInput.map(_.asHtml)
}

@govukTextarea(Textarea(
id = id,
name = name,
describedBy = Some(s"${id}-info"),
Expand All @@ -71,12 +80,11 @@
label = label.copy(forAttr = Some(id)),
hint = hint,
errorMessage = errorMessage,
formGroup = formGroup,
classes = toClasses("govuk-js-character-count", if (classes.isEmpty) "" else s" ${classes}"),
formGroup = formGroup.copy(
classes = Some(toClasses("govuk-character-count", formGroup.classes.getOrElse(""))),
attributes = dataAttributes ++ formGroup.attributes,
afterInput = Some(HtmlContent(characterCountMessagePlusOptionalAfterInput))
),
classes = toClasses("govuk-js-character-count", if (classes.isEmpty) "" else s" $classes"),
attributes = attributes
))
@govukHint(Hint(id=Some(id+"-info"),
classes = toClasses("govuk-character-count__message", if (countMessageClasses.isEmpty) "" else s" $countMessageClasses"),
content = customTextArea
))
</div>
))
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

@(params: Footer = Footer())
@import params._
<footer class="@toClasses("govuk-footer", classes)" role="contentinfo"@toAttributes(attributes)>
<footer class="@toClasses("govuk-footer", classes)" @toAttributes(attributes)>
<div class="@toClasses("govuk-width-container", containerClasses)">
@if(navigation.nonEmpty){
<div class="govuk-footer__navigation">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@

@(params: Header = Header())
@import params._
<header class="@toClasses("govuk-header", classes)" role="banner" data-module="govuk-header"@toAttributes(attributes)>
<header class="@toClasses("govuk-header", classes)" data-module="govuk-header"@toAttributes(attributes)>
<div class="govuk-header__container @containerClasses.getOrElse("govuk-width-container")">
<div class="govuk-header__logo">
<a href="@homepageUrl.getOrElse{/}" class="govuk-header__link govuk-header__link--homepage">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,14 @@
@govukHintAndErrorMessage(id, describedBy, hint, errorMessage)(inputSnippet)
}

@inputPrefix(prefix: Option[PrefixOrSuffix], hasPrefix: Boolean, hasSuffix: Boolean) = {@maybeWrapperStart(hasPrefix, hasSuffix)@maybePrefix(prefix, hasPrefix)}
@inputPrefix(prefix: Option[PrefixOrSuffix], hasPrefix: Boolean, hasSuffix: Boolean, hasAfterInput: Boolean) = {@maybeWrapperStart(hasPrefix, hasSuffix, hasAfterInput)@maybePrefix(prefix, hasPrefix)}

@maybePrefix(prefix: Option[PrefixOrSuffix], hasPrefix: Boolean) ={@if(hasPrefix) {<div class="@toClasses("govuk-input__prefix", prefix.get.classes)" aria-hidden="true"@toAttributes(prefix.get.attributes)>@prefix.get.content.asHtml</div>
}}

@maybeWrapperStart(hasPrefix: Boolean, hasSuffix: Boolean) = {@if(hasPrefix || hasSuffix) {<div class="govuk-input__wrapper"> }}
@maybeWrapperStart(hasPrefix: Boolean, hasSuffix: Boolean, hasAfterInput: Boolean) = {@if(hasPrefix || hasSuffix || hasAfterInput) {
<div class="@toClasses("govuk-input__wrapper", inputWrapper.classes.getOrElse(""))"@toAttributes(inputWrapper.attributes)>
}}

@inputSuffix(suffix: Option[PrefixOrSuffix], hasPrefix: Boolean, hasSuffix: Boolean) = {
@maybeSuffix(suffix, hasSuffix)@maybeWrapperEnd(hasPrefix: Boolean, hasSuffix: Boolean)
Expand All @@ -56,8 +58,8 @@
} @value.mapNonEmpty {value => value="@value" }@if(disabled.getOrElse(false)) { disabled}@if(describedBy.nonEmpty) { aria-describedby="@describedBy"}
} { attrs =>
@defining {
@autocomplete.mapNonEmpty { autocomplete => autocomplete="@autocomplete"}@pattern.mapNonEmpty { pattern => pattern="@pattern"}@inputmode.mapNonEmpty { inputmode => inputmode="@inputmode"}@toAttributes(attributes)
@autocomplete.mapNonEmpty { autocomplete => autocomplete="@autocomplete"}@pattern.mapNonEmpty { pattern => pattern="@pattern"}@inputmode.mapNonEmpty { inputmode => inputmode="@inputmode"}@autocapitalize.map {v => autocapitalize="@v"}@toAttributes(attributes)
} { otherAttrs =>
@(inputPrefix(params.prefix, params.prefix.nonEmpty, params.suffix.nonEmpty))<input class="@toClasses("govuk-input", classes, errorMessage.fold("")(_ => "govuk-input--error"))" @attrs@otherAttrs>@(inputSuffix(params.suffix, params.prefix.nonEmpty, params.suffix.nonEmpty))
@(inputPrefix(params.prefix, params.prefix.nonEmpty, params.suffix.nonEmpty, formGroup.afterInput.nonEmpty))<input class="@toClasses("govuk-input", classes, errorMessage.fold("")(_ => "govuk-input--error"))" @attrs@otherAttrs>@(inputSuffix(params.suffix, params.prefix.nonEmpty, params.suffix.nonEmpty))@formGroup.afterInput.map(_.asHtml)
}}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@

@blockLevel = @{items.isEmpty && {next.isDefined || previous.isDefined}}

<nav class="@toClasses("govuk-pagination", {if(blockLevel) "govuk-pagination--block" else ""}, classes)"
role="navigation" aria-label="@{landmarkLabel.getOrElse(messages("govuk.pagination.ariaLabel"))}"@toAttributes(attributes)>
<nav class="@toClasses("govuk-pagination", {if(blockLevel) "govuk-pagination--block" else ""}, classes)" aria-label="@{landmarkLabel.getOrElse(messages("govuk.pagination.ariaLabel"))}"@toAttributes(attributes)>
@previous.map { prev =>
@if(prev.href.nonEmpty) {
<div class="govuk-pagination__prev">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
content = label.content
))
@govukHintAndErrorMessage(id, describedBy, hint, errorMessage)(textareaSnippet)
@formGroup.afterInput.map(_.asHtml)
}

@textareaSnippet(describedBy: String) = {
Expand Down