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.

Sign up for GitHub

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

Jump to bottom

refactor(PasswordInput): use stateless pw input #2655

Closed
wants to merge 4 commits into from
Closed

refactor(PasswordInput): use stateless pw input #2655

wants to merge 4 commits into from

Conversation

emyarod
Copy link
Member

@emyarod emyarod commented May 9, 2019

This PR refactors <TextInput.PasswordInput> to consume the stateless <TextInput.ControlledPasswordInput>

refs #2489 #2427

Changelog

Changed

  • <TextInput.PasswordInput> now uses stateless <TextInput.ControlledPasswordInput>

@emyarod emyarod added this to the v10.3 milestone May 9, 2019
@emyarod emyarod requested a review from asudoh May 9, 2019 19:10
@emyarod emyarod added the package: react carbon-components-react label May 9, 2019
@emyarod emyarod mentioned this pull request May 9, 2019
Closed
@asudoh asudoh marked this pull request as ready for review May 10, 2019 00:32
asudoh
asudoh approved these changes May 10, 2019
View reviewed changes
Copy link
Contributor

@asudoh asudoh left a comment

Choose a reason for hiding this comment

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

LGTM 👍 - Thanks @emyarod!

@netlify
Copy link

netlify bot commented May 13, 2019 •
edited
Loading

Deploy preview for the-carbon-components ready!

Built with commit 4113c95

https://deploy-preview-2655--the-carbon-components.netlify.com

@netlify
Copy link

netlify bot commented May 13, 2019 •
edited
Loading

Deploy preview for carbon-components-react ready!

Built with commit 4113c95

https://deploy-preview-2655--carbon-components-react.netlify.com

@emyarod emyarod force-pushed the presentational-password-input branch from a6fab25 to b49e455 Compare May 14, 2019 13:26
@emyarod emyarod mentioned this pull request May 14, 2019
Closed
joshblack
joshblack suggested changes May 14, 2019
View reviewed changes
Copy link
Contributor

@joshblack joshblack left a comment

Choose a reason for hiding this comment

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

Is there a better name than PresentationalPasswordInput? It's just so long, did we decide on this convention already in other parts of the codebase?

@emyarod
Copy link
Member Author

emyarod commented May 14, 2019

@joshblack this was hand in hand with #2427 and the name change was determined last night. I will need to update this to follow the latest conventions set in that PR

@joshblack
Copy link
Contributor

Sounds good @emyarod 👍 Just commented in the other thread.

@jnm2377
Copy link
Contributor

jnm2377 commented May 23, 2019

Not sure if I missed something, but when I click the show password icon in the Fully controlled toggle password visibility story, it doesn't actually show the password. It always stays hidden.

@emyarod
Copy link
Member Author

emyarod commented May 23, 2019 •
edited
Loading

progress on this PR is stalled until we figure out how we want to proceed with #2730 #3028

@emyarod emyarod removed this from the v10.3 milestone May 28, 2019
@joshblack
Copy link
Contributor

bump @emyarod, is this going to be part of v10.4 now that the controlled stuff has been settled?

@emyarod
Copy link
Member Author

emyarod commented Jun 24, 2019

that's still the goal but it may end up in 10.5 with #3018

@stale
Copy link

stale bot commented Jul 24, 2019

We've marked this issue as stale because there hasn't been any activity for a couple of weeks. If there's no further activity on this issue in the next three days then we'll close it. You can keep the conversation going with just a short comment. Thanks for your contributions.

@stale stale bot added the status: inactive Will close if there's no further activity within a given time label Jul 24, 2019
@asudoh asudoh removed the status: inactive Will close if there's no further activity within a given time label Jul 24, 2019
@stale
Copy link

stale bot commented Aug 23, 2019

We've marked this issue as stale because there hasn't been any activity for a couple of weeks. If there's no further activity on this issue in the next three days then we'll close it. You can keep the conversation going with just a short comment. Thanks for your contributions.

@stale stale bot added the status: inactive Will close if there's no further activity within a given time label Aug 23, 2019
@asudoh asudoh removed the status: inactive Will close if there's no further activity within a given time label Aug 24, 2019
@jnm2377 jnm2377 added the status: inactive Will close if there's no further activity within a given time label Sep 10, 2019
@stale
Copy link

stale bot commented Sep 10, 2019

As there's been no activity since this issue was marked as stale, we are auto-closing it.

@stale stale bot closed this Sep 10, 2019
@jnm2377 jnm2377 reopened this Sep 10, 2019
@stale stale bot removed the status: inactive Will close if there's no further activity within a given time label Sep 10, 2019
@joshblack
Copy link
Contributor

Closing as stale 👍

@joshblack joshblack closed this Sep 18, 2019
@emyarod emyarod deleted the presentational-password-input branch August 30, 2021 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joshblack joshblack joshblack requested changes

@asudoh asudoh asudoh approved these changes

Assignees
No one assigned
Labels
package: react carbon-components-react status: merge conflict 🚫 status: one more review 👀 status: WIP 👩‍💻
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@emyarod @joshblack @jnm2377 @asudoh