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

Conversation

kyle-bowden
Copy link
Contributor

No description provided.

@kyle-bowden kyle-bowden force-pushed the PLATUI-2943_adr_password_field branch from 26a510c to 72b49aa Compare April 8, 2024 14:33
@platops-pr-bot
Copy link

@kyle-bowden kyle-bowden force-pushed the PLATUI-2943_adr_password_field branch from 72b49aa to 40f0c85 Compare April 8, 2024 15:01

* 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.
Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM extra context on this is "where we think that it would not be too troublesome / intensive for people to implement the markup themselves if they need to because it's a standard input and button some extra classes and attributes"

@kyle-bowden kyle-bowden force-pushed the PLATUI-2943_adr_password_field branch from edaf8f2 to 4eda4d5 Compare April 10, 2024 15:58
@ellamdav ellamdav changed the title PLATUI-2944: Add ADR to record decision on excluding password fields from govuk-frontend v5.3.9 PLATUI-2942: Upgrade library to govuk-frontend v5.3.0 Apr 11, 2024
@platops-pr-bot
Copy link

@ellamdav ellamdav marked this pull request as ready for review April 11, 2024 16:31
CHANGELOG.md Outdated Show resolved Hide resolved
}
}
@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 😅

@kyle-bowden kyle-bowden force-pushed the PLATUI-2943_adr_password_field branch from 3f3c459 to 21521f2 Compare April 12, 2024 12:29
@ellamdav ellamdav merged commit 4eee1ee into main Apr 15, 2024
1 check passed
@ellamdav ellamdav deleted the PLATUI-2943_adr_password_field branch April 15, 2024 09:18
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.

None yet

4 participants