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

Improve definition of currently focused area #5832

Merged
merged 3 commits into from Aug 18, 2020

Conversation

annevk
Copy link
Member

@annevk annevk commented Aug 17, 2020

This seems like an improvement, however the next step suggests a document does not necessarily have a focused area, which isn't acknowledged anywhere else?


/interaction.html ( diff )

This seems like an improvement, however the next step suggests a document does not necessarily have a focused area, which isn't acknowledged anywhere else?
Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

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

This LGTM as an editorial clarification. You could go further and turn "redo this step" into an explicit loop.

For the bigger problem: yes, it looks like it's not possible for the focused area of a document to be unset. The focus fixup rule will kick in and make it the viewport. I think the intention of this step is to change such cases into the iframe... but I wonder if it matches browsers. It doesn't help that the way this definition is used is pretty roundabout so it'd be hard to tell where the difference comes in.

@annevk annevk requested a review from domenic August 17, 2020 15:17
Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

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

LGTM, let's prefix with "Editorial: ". And maybe open a tracking issue for the null problem.

@annevk annevk merged commit 198a190 into master Aug 18, 2020
@annevk annevk deleted the annevk/currently-focused-area branch August 18, 2020 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

2 participants