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

Focus [autofocus] on navigation and frame load #169

Merged
merged 4 commits into from Feb 9, 2021
Merged

Conversation

seanpdoyle
Copy link
Contributor

@seanpdoyle seanpdoyle commented Feb 8, 2021

Focus autofocus on navigation and frame load

According to the <input> element's documentation for the
[autofocus] attribute:

A Boolean attribute which, if present, indicates that the input should
automatically have focus when the page has finished loading

Note: An element with the [autofocus] attribute may gain focus
before the DOMContentLoaded event is fired. No more than one element
in the document may have the [autofocus] attribute. If put on more
than one element, the first one with the attribute receives focus.

Focus the Snapshot that is connected to the DOM

During the rendering process, the PageRenderer replaces the current
Snapshot with the contents of the new Snapshot, while the
FrameRenderer extracts the contents of the new Snapshot and uses it to
replace the contents of the current Snapshot.

Instead of accepting a Snapshot as an argument,
Renderer.focusFirstAutofocusableElement() uses whichever Snapshot is
connected to the current document.

Co-authored-by: Javan Makhmali javan@basecamp.com

Test coverage to preserve turbo-frame attributes

Add test coverage for preserving turbo-frame attributes during render,
making sure that any element state is preserved while the contents are
modified.

Skip autofocus during previews

Pass the isPreview value to the View.renderPage() call, and then
restore the guard clause removed by a7d02f0.

@seanpdoyle seanpdoyle requested a review from javan February 8, 2021 18:58
According to the `<input>` element's documentation for the
`[autofocus]` attribute:

> A Boolean attribute which, if present, indicates that the input should
> automatically have focus when the page has finished loading
>
>
> Note: An element with the `[autofocus]` attribute may gain focus
> before the `DOMContentLoaded` event is fired. No more than one element
> in the document may have the `[autofocus]` attribute. If put on more
> than one element, the **first one with the attribute receives focus**.

[autofocus]: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/Input#htmlattrdefautofocus
seanpdoyle and others added 2 commits February 8, 2021 16:26
During the rendering process, the `PageRenderer` replaces the current
`Snapshot` with the contents of the new `Snapshot`, while the
`FrameRenderer` extracts the contents of the new Snapshot and uses it to
replace the contents of the current `Snapshot`.

Instead of accepting a `Snapshot` as an argument,
`Renderer.focusFirstAutofocusableElement()` uses whichever `Snapshot` is
connected to the current document.

Co-authored-by: Javan Makhmali <javan@basecamp.com>
Add test coverage for preserving turbo-frame attributes during render,
making sure that any element state is preserved while the contents are
modified.
Copy link
Contributor

@javan javan left a comment

Choose a reason for hiding this comment

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

Confirmed this fixes the issue I was seeing. Thank you!

Pass the `isPreview` value to the `View.renderPage()` call, and then
restore the guard clause removed by [a7d02f0][].

[a7d02f0]: a7d02f0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants