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

[docs][material-ui][Text Field] Keep the text caret position when clicking the visibility adornment #42595

Merged
merged 7 commits into from
Jun 26, 2024

Conversation

appleSimple
Copy link
Contributor

@appleSimple appleSimple commented Jun 10, 2024

when click password visibility button, text caret position change to very front.

Suggestion

I referred to Ant Design source, and they modified same issue before. (code here)

before

password-bug.mov

after

password-fix.mov

thanks for read!

@mui-bot
Copy link

mui-bot commented Jun 10, 2024

Netlify deploy preview

https://deploy-preview-42595--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against c09bad1

@zannager zannager added component: text field This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material labels Jun 10, 2024
@zannager zannager requested a review from DiegoAndai June 10, 2024 11:36
@DiegoAndai
Copy link
Member

Thanks for working on this @appleSimple! I think the change makes sense.

What do you think about this behaviour @zanivan?

@DiegoAndai DiegoAndai requested a review from zanivan June 14, 2024 14:02
Copy link
Contributor

@zanivan zanivan left a comment

Choose a reason for hiding this comment

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

Good catch—it makes a lot of sense! As I see, the common behavior is to fill in the password field, click the button to check if it's spelled correctly, or see what you've already inputted. IMO, positioning the caret this way aligns better with that behavior.

Could this be a breaking change? Developers might have already implemented local workarounds for this.

@DiegoAndai
Copy link
Member

Could this be a breaking change? Developers might have already implemented local workarounds for this.

This is only a change in the docs so it won't cause any breaking changes 👍🏼

Copy link
Member

@DiegoAndai DiegoAndai left a comment

Choose a reason for hiding this comment

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

@appleSimple may I ask you to run pnpm docs:typescript:formatted and commit the changes so the CI succeeds?

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jun 16, 2024
@appleSimple
Copy link
Contributor Author

Thank you for approvals guys! @zanivan @DiegoAndai .

I ran pnpm docs:typescript:formatted, and occured ci error so I ran pnpm dedupe that CircleCI was recommended.
(duplicated packages error)

But confilcted file was happened, What can I do?

@DiegoAndai
Copy link
Member

Hey @appleSimple, I recommend you do the following:

  1. Revert the pnpm-lock.yaml changes in your branch
  2. git checkout next
  3. git pull upstream next
  4. git checkout fix-prevent-caret-position-change
  5. git merge next
  6. pnpm install
  7. pnpm dedupe
  8. git push

Let me know if that works.

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jun 20, 2024
@appleSimple
Copy link
Contributor Author

oh, @DiegoAndai Thank you very much!!

Finally I'm done, please check it. 😄

Copy link
Member

@DiegoAndai DiegoAndai left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @appleSimple!

@DiegoAndai DiegoAndai merged commit 4780aeb into mui:next Jun 26, 2024
20 checks passed
@mnajdova mnajdova changed the title [material-ui][Text Field] when click password visibility button, text caret position change to very front. [docs][material-ui][Text Field] Improve the adornment input demo Jun 27, 2024
@mnajdova mnajdova changed the title [docs][material-ui][Text Field] Improve the adornment input demo [docs][material-ui][Text Field] Keep the text caret position when clicking the visibility adornment Jun 27, 2024
joserodolfofreitas pushed a commit to joserodolfofreitas/material-ui that referenced this pull request Jul 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: text field This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants