HMA-8827-password-input-view #191
Conversation
| @@ -16,6 +16,10 @@ Allowed headings: | |||
|
|
|||
| ## [Unreleased] | |||
|
|
|||
| ### Added | |||
|
|
|||
| * `PasswordInputView` component added with show/hide feature | |||
There was a problem hiding this comment.
I think we should mention the new requiresSequenceSpacing parameter here
| visualTransformation: VisualTransformation, | ||
| trailingIcon: @Composable() (() -> Unit)? = null, | ||
| textStyle: TextStyle = typography.body, | ||
| colors: TextFieldColors, |
There was a problem hiding this comment.
Can we move this above the parameters which have defaults set? That way, it is possible to create an instance of TextField without requiring named parameters if we're not overriding the defaults.
See here for the Google guidelines on parameter ordering: https://android.googlesource.com/platform/frameworks/support/+/androidx-main/compose/docs/compose-component-api-guidelines.md#parameters-order
| @@ -40,6 +41,7 @@ fun CurrencyInputView( | |||
| singleLine: Boolean = true, | |||
| enableDecimal: Boolean = true, | |||
| maxChars: Int? = null, | |||
| interactionSource: MutableInteractionSource = remember { MutableInteractionSource() }, | |||
There was a problem hiding this comment.
Do we need to expose this as a parameter for the CurrencyInputView? Will we ever need it? I think you only needed to add it to the password to do very custom behaviour for the focus colours, so I don't think we need to declare it here as we have the default parameter set on the TextInputView already.
| singleLine: Boolean = true, | ||
| numericOnly: Boolean = false, | ||
| maxChars: Int? = null, | ||
| interactionSource: MutableInteractionSource = remember { MutableInteractionSource() }, |
There was a problem hiding this comment.
Does this need to be a parameter here or can we just declare it inside the function? Will we ever need to pass in a new value for it?
| numericOnly: Boolean = false, | ||
| maxChars: Int? = null, | ||
| interactionSource: MutableInteractionSource = remember { MutableInteractionSource() }, | ||
| requiredSequencesSpacing: Boolean = false, |
There was a problem hiding this comment.
Don't we want the default to be true for passwords?
| @@ -33,6 +33,7 @@ val HmrcPink = Color(0xFFCA2B75) | |||
| val HmrcYellow = Color(0xFFFFBF47) | |||
| val HmrcAlwaysBlack = Color(0xFF0B0C0C) | |||
| val HmrcDonutBlue = Color(0xFF003078) | |||
| val hmrcTransparent = Color.Transparent | |||
There was a problem hiding this comment.
| val hmrcTransparent = Color.Transparent | |
| val HmrcTransparent = Color.Transparent |
| <string name="passwordTextInputView_hide">Hide</string> | ||
| <string name="passwordTextInputView_show">Show</string> |
There was a problem hiding this comment.
I don't think we want to include these in the actual component library package, but they can be declared in the sample package so we can use them in the examples.
| @@ -0,0 +1,48 @@ | |||
| /* | |||
| * Copyright 2023 HM Revenue & Customs | |||
There was a problem hiding this comment.
| * Copyright 2023 HM Revenue & Customs | |
| * Copyright 2024 HM Revenue & Customs |
| onInputValueChange = { placeholderValue = it }, | ||
| labelText = stringResource(id = R.string.password_input_placeholder_label), | ||
| hintText = stringResource(id = R.string.password_input_placeholder_hint), | ||
| placeholderText = stringResource(id = R.string.password_input_placeholder_placeholder), |
There was a problem hiding this comment.
This placeholder text isn't needed
| labelText = stringResource(id = R.string.password_input_example_3_label), | ||
| hintText = stringResource(id = R.string.password_input_example_3_hint), | ||
| ) | ||
| } |
There was a problem hiding this comment.
Could we add an example that showcases all the accessibility stuff so it can be tested, like adding different content descriptions and button text?
📝 Description
HMA-8827
🛠 Testing Notes
Check text input view, currency input view, password input view
📸 Screenshots