-
Notifications
You must be signed in to change notification settings - Fork 20
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
Add the password input component #4176
Conversation
2d041da
to
8f1a5bd
Compare
8f1a5bd
to
de1f763
Compare
de1f763
to
0e5e546
Compare
0e5e546
to
eb60fc8
Compare
eb60fc8
to
5b98fc9
Compare
5b98fc9
to
d8e90b1
Compare
d8e90b1
to
31765a1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Just a few minor things to look at.
|
||
label_text ||= t("components.password_input.label") | ||
|
||
# This overrides the 'Show/Hide' button text, but this button text is getting overwritten by GOVUK Frontend JS currently when you interact with the button. Therefore this functionality is not being exposed in the docs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Worth asking the Design System team if it's possible/planned to be able to override the text values in https://github.com/alphagov/govuk-frontend/blob/main/packages/govuk-frontend/src/govuk/components/password-input/password-input.mjs#L222-L231
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@andysellick It looks possible, so I'll implement it tomorrow 👍 https://gds.slack.com/archives/CAF8JA25U/p1725376708644069?thread_ts=1725374973.976699&cid=CAF8JA25U
app/views/govuk_publishing_components/components/_password_input.html.erb
Outdated
Show resolved
Hide resolved
app/views/govuk_publishing_components/components/_password_input.html.erb
Show resolved
Hide resolved
app/views/govuk_publishing_components/components/_password_input.html.erb
Outdated
Show resolved
Hide resolved
app/views/govuk_publishing_components/components/_password_input.html.erb
Outdated
Show resolved
Hide resolved
app/views/govuk_publishing_components/components/docs/password_input.yml
Outdated
Show resolved
Hide resolved
app/views/govuk_publishing_components/components/docs/password_input.yml
Outdated
Show resolved
Hide resolved
app/views/govuk_publishing_components/components/_password_input.html.erb
Show resolved
Hide resolved
ea67340
to
57636c4
Compare
57636c4
to
6f14bc6
Compare
Thanks for reviewing @andysellick 👍 I've made every change requested, so this should be ready for another review. |
it "renders a password input component correctly by default" do | ||
render_component({}) | ||
puts rendered |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Debug?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work. One bit of debug to remove, and obviously squash the commits, but happy to approve ahead of that 👍
6f14bc6
to
398f4f7
Compare
398f4f7
to
1fa5ef5
Compare
1fa5ef5
to
eacb157
Compare
What / Why
Show/Hide
button text to be overwritten. However, the button text gets changed by the GOVUK Frontend JavaScript on click. Therefore even if we change the text in the HTML, it gets overwritten as soon as you click the button. Therefore I've added the ability to change the button text in the template file, but I haven't made it known in theyml
docs that it's a feature.Visual Changes