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

feat(InputField): add show/hide button for password field type #2006

Merged
merged 1 commit into from
Jul 5, 2024

Conversation

booc0mtaco
Copy link
Contributor

@booc0mtaco booc0mtaco commented Jul 5, 2024

  • add tests and snapshots
  • implement show/hide button to appear after any inputWithin elements

Test Plan:

  • Wrote automated tests
  • CI tests / new tests are not applicable
  • Manually tested my changes, but I want to keep the details secret
  • Created and used an alpha publish
  • Manually tested my changes, and here are the details:

@booc0mtaco booc0mtaco requested a review from a team July 5, 2024 14:28
@booc0mtaco
Copy link
Contributor Author

booc0mtaco commented Jul 5, 2024

  • TODO: add in change to icon on click to toggle the open/close eye-con 👁️

Copy link

codecov bot commented Jul 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.53%. Comparing base (3eb9077) to head (4b43089).

Additional details and impacted files
@@           Coverage Diff           @@
##             next    #2006   +/-   ##
=======================================
  Coverage   97.52%   97.53%           
=======================================
  Files         108      108           
  Lines        2545     2555   +10     
  Branches      635      640    +5     
=======================================
+ Hits         2482     2492   +10     
  Misses         61       61           
  Partials        2        2           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

github-actions bot commented Jul 5, 2024

size-limit report 📦

Path Size
components 87.7 KB (+0.26% 🔺)
styles 27.65 KB (0%)

- toggle button label and icon based on current state
- implement show/hide button to appear after any inputWithin elements
- add tests and snapshots
@booc0mtaco booc0mtaco force-pushed the aholloway/EDS-1267 branch from 536ad64 to 4b43089 Compare July 5, 2024 15:00
@booc0mtaco
Copy link
Contributor Author

Screen.Recording.2024-07-05.at.10.14.04.mov

Copy link
Contributor

@jeremiah-clothier jeremiah-clothier left a comment

Choose a reason for hiding this comment

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

LGTM! More a curiosity question, how does copy+paste work when the password is hidden

@@ -292,12 +297,24 @@ export const InputField: InputFieldType = forwardRef(
ref={ref}
required={required}
status={shouldRenderError ? 'critical' : status}
type={type}
type={isPasswordVisible ? 'text' : type}
Copy link
Contributor

Choose a reason for hiding this comment

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

Not having the type="password" may mess with the usage of autocomplete, but since we're using Google Login for Render I don't think this is an issue

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/password#allowing_autocomplete

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's true, and there are a few safeguards for the scenario:

  • this switching to text only applies after interacting with the button, so by default it's a regular password field
  • I tested locally and I get the 1Password prompt (which nicely overlays the show/hide button) when set to password

My gut on the normal usage here is that a person would usually either paste into the password field, use the UI (when still set to type "password"), or reveal the text IFF they want to type it manually and see what they are typing/pasting or verify it after the fact.

@booc0mtaco
Copy link
Contributor Author

@jeremiah-clothier paste works as you'd expect, as does copy. We don't do any tracking of what's typed and replacing it with ••• in the field, so the browser handles converting the typed content to the actual text

@booc0mtaco booc0mtaco merged commit 52d9ca0 into next Jul 5, 2024
10 checks passed
@booc0mtaco booc0mtaco deleted the aholloway/EDS-1267 branch July 5, 2024 18:10
This was referenced Jul 12, 2024
booc0mtaco added a commit that referenced this pull request Jul 15, 2024
## [15.1.0](v15.0.1...v15.1.0) (2024-07-15)

[Storybook](https://61313967cde49b003ae2a860-qztphlqyid.chromatic.com/)

### Features

* add runtime warning/errors to components ([#2007](#2007)) ([661130b](661130b))
* **InputField:** add show/hide button for password field type ([#2006](#2006)) ([52d9ca0](52d9ca0))
* **Modal:** add height property to modal API ([#2011](#2011)) ([8d0c68f](8d0c68f))


### Bug Fixes

* **Icon:** update pencil icon to latest design ([#2016](#2016)) ([cb8d1a7](cb8d1a7))
* **Link:** apply font weight to standalone sizes ([#2015](#2015)) ([2e47271](2e47271))
* **Select:** expose generic types to allow by to pass type checks ([#2008](#2008)) ([421c91b](421c91b))
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.

2 participants