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

Placeholder changes to input value in readonly input. #34040

Merged
merged 8 commits into from
Jun 3, 2021

Conversation

rajesh2kumar96
Copy link
Contributor

@rajesh2kumar96 rajesh2kumar96 commented May 20, 2021

Fixes #34017

@ffoodd
Copy link
Member

ffoodd commented May 20, 2021

Thanks! Could you check there're no other occurrencies of such readonly inputs missing a value, either in the docs or the examples, please? If not, we'll take care of it 🙂

@coliff
Copy link
Contributor

coliff commented May 20, 2021

@ffoodd - yes, I think changes to the CSS are needed too. If you change the placeholder to a value then the displayed 'Readonly input here...' text is not the lighter grey as the Disabled inputs above.

Screenshot here by changing the HTML elements in Dev Tools (Edge 90 for Windows):
image

@ffoodd
Copy link
Member

ffoodd commented May 26, 2021

@coliff AFAIC that's acceptable since:

  • readonly is meant to be read, so better contrast are fine,
  • readonly and disabled aren't the same, so this helps reduce the confusion IMHO.
    However that's more of a UX decision. Maybe some accessibility insights too, @patrickhlauke?

@rajesh2kumar96 Were you able to check if there were other readonly inputs missing a value, both in our docs and in examples? If you can't, I'll try to.

@patrickhlauke
Copy link
Member

going purely by WCAG, nominally disabled fields / controls are exempted from 4.5:1 (regular) / 3:1 (large) contrast requirements. readonly fields aren't. even for disabled fields, a ratio of at least 3:1 or better is still best practice, the closer it can get to 4.5:1, the better. beyond that, it's down to aesthetic choices.

@rajesh2kumar96
Copy link
Contributor Author

@coliff AFAIC that's acceptable since:

  • readonly is meant to be read, so better contrast are fine,
  • readonly and disabled aren't the same, so this helps reduce the confusion IMHO.
    However that's more of a UX decision. Maybe some accessibility insights too, @patrickhlauke?

@rajesh2kumar96 Were you able to check if there were other readonly inputs missing a value, both in our docs and in examples? If you can't, I'll try to.

I will do this.

@rajesh2kumar96
Copy link
Contributor Author

@ffoodd I changed readonly inputs with value. Please check.

@mdo mdo merged commit 071a288 into twbs:main Jun 3, 2021
marvin-hinkley-vortx pushed a commit to Vortx-Inc/bootstrap that referenced this pull request Aug 18, 2021
* placeholder changes to input value

* Fix content and add value to readOnly input

* add value in readonly input

* Update site/content/docs/5.0/forms/form-control.md

Co-authored-by: Rajesh Kumar <https://github.com/rajesh2kumar96>
Co-authored-by: Patrick H. Lauke <redux@splintered.co.uk>
Co-authored-by: Mark Otto <otto@github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Docs: Readonly input example should have a value
5 participants