Skip to content

Commit

Permalink
Restore focus when transposing Permanent Elements
Browse files Browse the repository at this point in the history
Problem
---

When an interactive element like an `<input>`, `<select>`, `<textarea>`,
or `<button>` has an `[id]` attribute and is marked as
`[data-turbo-permanent]`, the element and its internal state is
preserved _except_ for its focus state.

Imagine a search form:

```html
<form>
  <label for="search">Search</label>
  <input type="search" id="search" name="q" data-turbo-permanent>
</form>
```

When typing a query into the box and submitting the `<form>` by
<kbd>enter</kbd>, the request is submitted, the page transitions, the
element retains its `[value]` attribute, but focus is lost.

Solution
---

When transposing a permanent element during a `Renderer` descendant's
(e.g. [PageRenderer](./src/core/drive/page_renderer.ts),
[ErrorRenderer](./src/core/drive/error_renderer.ts), or
[FrameRenderer](./src/core/frames/frame_renderer.ts)) render cycle,
capture the `document.activeElement`.

Once the permanent element is inserted into the new page, use that
retained reference to re-focus the element.
  • Loading branch information
seanpdoyle committed May 2, 2022
1 parent 933419b commit 347a0b0
Show file tree
Hide file tree
Showing 11 changed files with 119 additions and 14 deletions.
23 changes: 18 additions & 5 deletions src/core/bardo.ts
@@ -1,22 +1,34 @@
import { PermanentElementMap } from "./snapshot"

export interface BardoDelegate {
enteringBardo(currentPermanentElement: Element, newPermanentElement: Element): void
leavingBardo(currentPermanentElement: Element): void
}

export class Bardo {
readonly permanentElementMap: PermanentElementMap
readonly delegate: BardoDelegate

static preservingPermanentElements(permanentElementMap: PermanentElementMap, callback: () => void) {
const bardo = new this(permanentElementMap)
static preservingPermanentElements(
delegate: BardoDelegate,
permanentElementMap: PermanentElementMap,
callback: () => void
) {
const bardo = new this(delegate, permanentElementMap)
bardo.enter()
callback()
bardo.leave()
}

constructor(permanentElementMap: PermanentElementMap) {
constructor(delegate: BardoDelegate, permanentElementMap: PermanentElementMap) {
this.delegate = delegate
this.permanentElementMap = permanentElementMap
}

enter() {
for (const id in this.permanentElementMap) {
const [, newPermanentElement] = this.permanentElementMap[id]
const [currentPermanentElement, newPermanentElement] = this.permanentElementMap[id]
this.delegate.enteringBardo(currentPermanentElement, newPermanentElement)
this.replaceNewPermanentElementWithPlaceholder(newPermanentElement)
}
}
Expand All @@ -26,6 +38,7 @@ export class Bardo {
const [currentPermanentElement] = this.permanentElementMap[id]
this.replaceCurrentPermanentElementWithClone(currentPermanentElement)
this.replacePlaceholderWithPermanentElement(currentPermanentElement)
this.delegate.leavingBardo(currentPermanentElement)
}
}

Expand All @@ -49,7 +62,7 @@ export class Bardo {
}

get placeholders(): HTMLMetaElement[] {
return [...document.querySelectorAll("meta[name=turbo-permanent-placeholder][content]")] as any
return [...document.querySelectorAll<HTMLMetaElement>("meta[name=turbo-permanent-placeholder][content]")]
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/core/drive/page_renderer.ts
Expand Up @@ -12,13 +12,13 @@ export class PageRenderer extends Renderer<HTMLBodyElement, PageSnapshot> {
get reloadReason(): ReloadReason {
if (!this.newSnapshot.isVisitable) {
return {
reason: "turbo_visit_control_is_reload"
reason: "turbo_visit_control_is_reload",
}
}

if (!this.trackedElementsAreIdentical) {
return {
reason: "tracked_element_mismatch"
reason: "tracked_element_mismatch",
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/core/native/browser_adapter.ts
Expand Up @@ -8,7 +8,7 @@ import { uuid, dispatch } from "../../util"
export type ReloadReason = StructuredReason | undefined
interface StructuredReason {
reason: string
context?: {[key: string]: any}
context?: { [key: string]: any }
}

export class BrowserAdapter implements Adapter {
Expand Down Expand Up @@ -54,8 +54,8 @@ export class BrowserAdapter implements Adapter {
return this.reload({
reason: "request_failed",
context: {
statusCode
}
statusCode,
},
})
default:
return visit.loadResponse()
Expand Down
25 changes: 22 additions & 3 deletions src/core/renderer.ts
@@ -1,4 +1,4 @@
import { Bardo } from "./bardo"
import { Bardo, BardoDelegate } from "./bardo"
import { Snapshot } from "./snapshot"
import { ReloadReason } from "./native/browser_adapter"

Expand All @@ -7,13 +7,14 @@ type ResolvingFunctions<T = unknown> = {
reject(reason?: any): void
}

export abstract class Renderer<E extends Element, S extends Snapshot<E> = Snapshot<E>> {
export abstract class Renderer<E extends Element, S extends Snapshot<E> = Snapshot<E>> implements BardoDelegate {
readonly currentSnapshot: S
readonly newSnapshot: S
readonly isPreview: boolean
readonly willRender: boolean
readonly promise: Promise<void>
private resolvingFunctions?: ResolvingFunctions<void>
private activeElement: Element | null = null

constructor(currentSnapshot: S, newSnapshot: S, isPreview: boolean, willRender = true) {
this.currentSnapshot = currentSnapshot
Expand Down Expand Up @@ -60,7 +61,7 @@ export abstract class Renderer<E extends Element, S extends Snapshot<E> = Snapsh
}

preservingPermanentElements(callback: () => void) {
Bardo.preservingPermanentElements(this.permanentElementMap, callback)
Bardo.preservingPermanentElements(this, this.permanentElementMap, callback)
}

focusFirstAutofocusableElement() {
Expand All @@ -70,6 +71,24 @@ export abstract class Renderer<E extends Element, S extends Snapshot<E> = Snapsh
}
}

// Bardo delegate

enteringBardo(currentPermanentElement: Element) {
if (this.activeElement) return

if (currentPermanentElement.contains(this.currentSnapshot.activeElement)) {
this.activeElement = this.currentSnapshot.activeElement
}
}

leavingBardo(currentPermanentElement: Element) {
if (currentPermanentElement.contains(this.activeElement) && this.activeElement instanceof HTMLElement) {
this.activeElement.focus()

this.activeElement = null
}
}

get connectedSnapshot() {
return this.newSnapshot.isConnected ? this.newSnapshot : this.currentSnapshot
}
Expand Down
2 changes: 1 addition & 1 deletion src/core/session.ts
Expand Up @@ -128,7 +128,7 @@ export class Session
})
} else {
this.adapter.pageInvalidated({
reason: "turbo_disabled"
reason: "turbo_disabled",
})
}
}
Expand Down
4 changes: 4 additions & 0 deletions src/core/snapshot.ts
Expand Up @@ -5,6 +5,10 @@ export class Snapshot<E extends Element = Element> {
this.element = element
}

get activeElement() {
return this.element.ownerDocument.activeElement
}

get children() {
return [...this.element.children]
}
Expand Down
8 changes: 8 additions & 0 deletions src/tests/fixtures/frames/hello.html
Expand Up @@ -12,6 +12,14 @@ <h1>Hello</h1>
<h2>Hello from a frame</h2>

<a href="/src/tests/fixtures/frames.html">Navigate #hello</a>

<form>
<input id="permanent-input-in-frame" type="text" name="query" placeholder="Permanent input in frame" data-turbo-permanent>
</form>

<form id="permanent-form-in-frame" data-turbo-permanent>
<input id="permanent-descendant-input-in-frame" type="text" name="query" placeholder="Permanent descendant input in frame">
</form>
</turbo-frame>
</body>
</html>
20 changes: 20 additions & 0 deletions src/tests/fixtures/rendering.html
Expand Up @@ -44,10 +44,30 @@ <h1>Rendering</h1>
</section>
<div id="permanent" data-turbo-permanent>Rendering</div>

<form>
<input id="permanent-input" type="text" name="query" placeholder="Permanent input" data-turbo-permanent>
</form>

<form id="permanent-form" data-turbo-permanent>
<input id="permanent-descendant-input" type="text" name="query" placeholder="Permanent descendant input">
</form>

<turbo-frame id="frame">
<div id="permanent-in-frame" data-turbo-permanent>Rendering</div>
</turbo-frame>

<turbo-frame id="hello">
<form action="/__turbo/redirect">
<input type="hidden" name="path" value="/src/tests/fixtures/frames/hello.html">
<input id="permanent-input-in-frame" type="text" name="query" placeholder="Permanent input in frame" data-turbo-permanent>
</form>

<form id="permanent-form-in-frame" action="/__turbo/redirect" data-turbo-permanent>
<input type="hidden" name="path" value="/src/tests/fixtures/frames/hello.html">
<input id="permanent-descendant-input-in-frame" type="text" name="query" placeholder="Permanent descendant input in frame" data-turbo-permanent>
</form>
</turbo-frame>

<video id="permanent-video" data-turbo-permanent>
<source src="/src/tests/fixtures/video.webm" type="video/webm">
<source src="/src/tests/fixtures/video.mp4" type="video/mp4">
Expand Down
2 changes: 2 additions & 0 deletions src/tests/functional/form_submission_tests.ts
Expand Up @@ -25,6 +25,8 @@ export class FormSubmissionTests extends TurboDriveTestCase {

this.assert.equal(await this.getAlertText(), "Are you sure?")
await this.acceptAlert()
await this.nextEventNamed("turbo:load")

this.assert.ok(await this.formSubmitStarted)
this.assert.equal(await this.pathname, "/src/tests/fixtures/one.html")
}
Expand Down
35 changes: 35 additions & 0 deletions src/tests/functional/rendering_tests.ts
Expand Up @@ -171,6 +171,22 @@ export class RenderingTests extends TurboDriveTestCase {
this.assert(await permanentElement.equals(await this.permanentElement))
}

async "test restores focus during page rendering when transposing the activeElement"() {
await this.clickSelector("#permanent-input")
await this.pressEnter()
await this.nextBody

this.assert.ok(await this.selectorHasFocus("#permanent-input"), "restores focus after page loads")
}

async "test restores focus during page rendering when transposing an ancestor of the activeElement"() {
await this.clickSelector("#permanent-descendant-input")
await this.pressEnter()
await this.nextBody

this.assert.ok(await this.selectorHasFocus("#permanent-descendant-input"), "restores focus after page loads")
}

async "test preserves permanent elements within turbo-frames"() {
let permanentElement = await this.querySelector("#permanent-in-frame")
this.assert.equal(await permanentElement.getVisibleText(), "Rendering")
Expand All @@ -191,6 +207,25 @@ export class RenderingTests extends TurboDriveTestCase {
this.assert.equal(await permanentElement.getVisibleText(), "Rendering")
}

async "test restores focus during turbo-frame rendering when transposing the activeElement"() {
await this.clickSelector("#permanent-input-in-frame")
await this.pressEnter()
await this.nextBeat

this.assert.ok(await this.selectorHasFocus("#permanent-input-in-frame"), "restores focus after page loads")
}

async "test restores focus during turbo-frame rendering when transposing a descendant of the activeElement"() {
await this.clickSelector("#permanent-descendant-input-in-frame")
await this.pressEnter()
await this.nextBeat

this.assert.ok(
await this.selectorHasFocus("#permanent-descendant-input-in-frame"),
"restores focus after page loads"
)
}

async "test preserves permanent element video playback"() {
let videoElement = await this.querySelector("#permanent-video")
await this.clickSelector("#permanent-video-button")
Expand Down
4 changes: 4 additions & 0 deletions src/tests/helpers/functional_test_case.ts
Expand Up @@ -71,6 +71,10 @@ export class FunctionalTestCase extends InternTestCase {
return this.remote.getActiveElement().then((activeElement) => activeElement.type("\uE004")) // TAB
}

async pressEnter(): Promise<void> {
return this.remote.getActiveElement().then((activeElement) => activeElement.type("\uE006")) // ENTER
}

async outerHTMLForSelector(selector: string): Promise<string> {
const element = await this.remote.findByCssSelector(selector)
return this.evaluate((element) => element.outerHTML, element)
Expand Down

0 comments on commit 347a0b0

Please sign in to comment.