-
Notifications
You must be signed in to change notification settings - Fork 22.6k
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
Update autocapitalize props #30734
Update autocapitalize props #30734
Conversation
Preview URLs
(comment last updated: 2023-12-04 12:00:31) |
- `none`: No automatic capitalization. | ||
- `sentences` (default): Capitalize the first letter of each sentence. | ||
- `none` or `off`: No automatic capitalization. | ||
- `sentences` or `on` (default): Capitalize the first letter of each sentence. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- `sentences` or `on` (default): Capitalize the first letter of each sentence. | |
- `sentences` or `on`: Capitalize the first letter of each sentence. |
default
The user agent and input method should use make their own determination of whether or not to enable autocapitalization.
ref: https://html.spec.whatwg.org/multipage/interaction.html#autocapitalization
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi there @ryo-manba , and thank you for your contribution!
I think your change is correct, but there are a few other problems with the autocapitalize
documentation as I see it, which you could also consider adding/updating. I wrote up a quick test case at https://autocapitalize-test.glitch.me/. From my research, and from testing across different desktop and mobile browsers, I observed the following:
- As far as I can tell, it doesn't work on desktop browsers.
- It does work across mobile browsers: I tested Firefox Android, Chrome Android, and Safari iOS. All of the values work as expected.
- The mobile browsers do differ in terms of what their default behaviors is when no
autocapitalize
is specified. On Chrome Android and Safari iOS the default behavior ison
/sentences
. On Firefox Android the default isoff
/none
. This is worth documenting. - It is in a spec and supported across multiple browsers, so you can get rid of the
{{non-standard_inline}}
macro call and the mention of non-standard in the text. Also maybe update "used by iOS Safari" to "used by mobile browsers"? - Also, whatever changes you make here should also be made at https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input#autocapitalize (also move it out fo the non-standard list to the standard list?) and https://developer.mozilla.org/en-US/docs/Web/HTML/Element/textarea (it is covered here at all, so add it?)
- On the
<input>
page, you should document this bit from the spec: "The autocapitalize attribute never causes autocapitalization to be enabled for input elements whose type attribute is in one of the URL, Email, or Password states. (This behavior is included in the used autocapitalization hint algorithm below.)"
What do you think? Are you OK to work on these changes?
The browser compat data also needs updating to mark desktop browsers as not supporting it, and add it to element where it isn't included? This can be found at https://github.com/mdn/browser-compat-data. I appreciate that this is really another task, not part of this same one.
@chrisdavidmills |
@chrisdavidmills |
I think the easiest way to solve this is to get this PR locally (using More efficient than trial and error. The Github CLI is very useful for such cases. |
- : Automatically capitalize every character. | ||
|
||
> **Note:** When `autocapitalize` is set on a `<form>`, it controls the autocapitalization behavior of all contained {{htmlelement("input")}} (except `url`, `email`, and `password` types) and {{htmlelement("textarea")}} elements, overriding any `autocapitalize` values set on contained elements. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[mdn-linter] reported by reviewdog 🐶
@teoli2003 Yup, I'm going to tackle the linting errors now, by pulling it locally, don't worry. I was making some changes to the text anyway. |
@teoli2003 Sorry, can you help me with this? I'm really struggling to get it to work. I don't use the github cli, so your commands won't work for me. And I can't figure out what the magical incantation is to get this to work in raw git commands. |
Replace upstream by origin in the following if you kept the default upstream remote name, but this should work:
|
@teoli2003 great, so this worked! Thanks. (I'm pretty sure this is what I was doing before, not sure why it wasn't working, but hey) So now I've run prettier, added, committed, and I'm trying to push the changes back to this PR. How do I do that successfully?
I really hate git sometimes. |
So, it worked when I did:
followed by your changes and then:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks so much for your great work on the content @ryo-manba! It was very close to perfect; I just decided to make some fixes to the grammar before merging it.
Also a huge thanks to @teoli2003 for helping me with my git issues. I have taken note of your method, and I will look into the GitHub CLI too.
* Update autocapitalize props * Apply suggestions from code review * Update autocapitalize attribute documentation for mobile browser compatibility * Update textarea and input documentation for autocapitalize attribute * fix attribute types * fix: link * Tweaks to <input> text * Tweaks to <textarea> * Couple more tweaks * Small tweak * Tweak <form> * Fix note boxes * Test * Remove test --------- Co-authored-by: Chris Mills <[email protected]> Co-authored-by: Jean-Yves Perrier <[email protected]>
Just a nitpick, but now that this PR has been merged, could you please delete the Because currently, when creating a PR using the GitHub website, contributors end up with an autogenerated branch named |
Description
According to HTML Standard, it also accepts the following off and on keywords.
https://html.spec.whatwg.org/multipage/interaction.html#autocapitalization
This is a follow-up to the following PR.