Skip to content

Ensure that Element.focus() centers the element.#41029

Merged
domenic merged 1 commit into
web-platform-tests:masterfrom
dlrobertson:dom-focus
Jul 19, 2023
Merged

Ensure that Element.focus() centers the element.#41029
domenic merged 1 commit into
web-platform-tests:masterfrom
dlrobertson:dom-focus

Conversation

@dlrobertson

Copy link
Copy Markdown
Member

Element focus should behave similar to
scrollIntoView({block: "center", inline: "center"}) when the element is not in view.

@wpt-pr-bot

Copy link
Copy Markdown
Collaborator

There are no reviewers for this pull request. Please reach out on the chat room to get help with this. Thank you!

Comment thread focus/focus-centers-element.html Outdated
let scrollPos = test0.scrollTop;

// Ensure that both scroll positions are within +/- 1
assert_true(focusPos <= scrollPos + 1 && focusPos >= scrollPos - 1,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Comment thread focus/focus-centers-element.html Outdated
"focus() is within +/- 1 of a centered scrollIntoView()");
}, "Element.focus() center in the inline direction");

promise_test(async (t) => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Doesn't this test subsume the previous two?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, I'll remove it. Originally added it just for completeness

Comment thread focus/focus-centers-element.html Outdated
"focus() is within +/- 1 of a centered scrollIntoView()");
}, "Element.focus() center in the block direction");

promise_test(async (t) => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm still confused why there are two tests here. They are identical up until the assert, right? Why not just include both asserts in one test?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This was an artifact of test development. Updated to use just the test that asserts both directions are centered.

Element focus should behave similar to
`scrollIntoView({block: "center", inline: "center"})` when the element
is not in view.
@domenic domenic merged commit 58c50ca into web-platform-tests:master Jul 19, 2023
@dlrobertson dlrobertson deleted the dom-focus branch July 19, 2023 10:59
@dlrobertson

Copy link
Copy Markdown
Member Author

Thanks for the review!

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.

4 participants