Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implement dialog initial focus proposal #8199
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
Implement dialog initial focus proposal #8199
Changes from all commits
2394e21
fd9c31d
b52e9b6
8da9666
2070d76
50c821f
844c49f
c15e1fc
4727fc0
8ff0298
da545c2
7003f1f
1ca7cbb
7dae27e
5907549
b09e68c
136592c
1f0a278
c6de43e
e4046aa
f60b5e2
3f56f1f
000aea4
30683ac
c95c97b
133d817
76ed03e
572a1c8
7585ed6
9fe975a
ca5d8e9
1763b9c
ee1a835
b9c63ca
494d638
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
I'm sorry, this is totally on me, but I forgot why we changed this from the example at https://github.com/whatwg/html/wiki/dialog--initial-focus,-a-proposal#example . @scottaohara I vaguely remember you being involved?
I'm not very happy with this example because it's just hard to believe the web developer would put the button before the input. The example at https://github.com/whatwg/html/wiki/dialog--initial-focus,-a-proposal#example is more believable. If there's something wrong with my initial version (I vaguely recall there was), could we try to come up with a more believable example here?
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.
Ah, I found the conversation. It's at #8199 (comment) (collapsed by default). I guess @annevk didn't like the example because it relied on the focusablity of readonly controls? I don't think that's so strange. The point of readonly (as opposed to disabled) is explicitly to be focusable/selectable. E.g. it's what I use in the second textbox in https://domenic.github.io/rewrapper/ .
@annevk , are you OK reverting this to the previous example?
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.
agreed @domenic, far prefer the original example as this new one doesn't make sense. the original example is something i can point to many instances of in microsoft products.
i'm confused by the original comment, indicating that Firefox did not autofocus readonly controls. testing today, focus goes to the readonly text field just fine (chrome and firefox), as i'd expect. So maybe Firefox has reversed course to match up with other implementations since october?
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.
Ok I tentatively reverted back to the previous example
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.
Alright, fair enough.
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.
Is this intended to be non-normative? If so, don't use "should".
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.
I intended this to be normative when I wrote it. Do you think it's a bad normative requirement, e.g. because it's too vague? I'm not sure on the precedent there.
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.
Sorry for the delay. Yes, I think it's not a good requirement because it's vague and not testable. Further, the wording "authors should work to ensure" makes the requirement about the work the author does, which is a bit off.
If it's not testable, I think making it non-normative is better. e.g. s/should work/are encouraged/
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.
I don't see any remaining normative text in the section we're talking about, so I think this is resolved?
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.
This paragraph is still there and still says "authors should work to ensure".
Perhaps we could just change it to "authors are encouraged"?