kopia lustrzana https://github.com/wagtail/wagtail
Do not swap sessions list HTML until focus has left the container
Prevent disrupting the flow of keyboard users when they are focusing on the sessions list.pull/12185/head
Sage Abdullah
Thibaud Colas
rodzic
fba4389f08
commit
c46dd53254
|
|
@ -1643,4 +1643,328 @@ describe('SwapController', () => {
|
|||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('deferring the content update until the focus is out of the target container', () => {
|
||||
const getMockResultsWithButtons = (total) =>
|
||||
getMockResults({ total }) +
|
||||
`<button id="button1">Focusable 1</button><button id="button2">Focusable 2</button>`;
|
||||
|
||||
beforeEach(() => {
|
||||
document.body.innerHTML = /* html */ `
|
||||
<main>
|
||||
<form
|
||||
action="/path/to/editing-sessions/"
|
||||
method="get"
|
||||
data-controller="w-swap"
|
||||
data-action="submit->w-swap#submitLazy:prevent"
|
||||
data-w-swap-defer-value="true"
|
||||
data-w-swap-target-value="#editing-sessions"
|
||||
>
|
||||
<input name="title" type="text"/>
|
||||
<input name="type" type="hidden" value="some-type" />
|
||||
<button id="submit" type="submit">Submit</button>
|
||||
</form>
|
||||
<div id="editing-sessions">
|
||||
<div>
|
||||
<button id="button1">Focusable 1</button>
|
||||
<button id="button2">Focusable 2</button>
|
||||
</div>
|
||||
</div>
|
||||
</main>
|
||||
`;
|
||||
});
|
||||
|
||||
it('should wait until the target loses focus and continue execution immediately after', async () => {
|
||||
const onSuccess = new Promise((resolve) => {
|
||||
document.addEventListener('w-swap:success', resolve);
|
||||
});
|
||||
|
||||
const beginEventHandler = jest.fn();
|
||||
document.addEventListener('w-swap:begin', beginEventHandler);
|
||||
|
||||
fetch.mockResponseSuccessText(getMockResultsWithButtons(5));
|
||||
|
||||
expect(handleError).not.toHaveBeenCalled();
|
||||
expect(global.fetch).not.toHaveBeenCalled();
|
||||
|
||||
document.getElementById('submit').click();
|
||||
|
||||
// focus on an element inside the target container
|
||||
document.getElementById('button1').focus();
|
||||
|
||||
expect(beginEventHandler).not.toHaveBeenCalled();
|
||||
|
||||
jest.runAllTimers(); // submit is debounced
|
||||
|
||||
// should fire a begin event before the request is made
|
||||
expect(beginEventHandler).toHaveBeenCalledTimes(1);
|
||||
expect(beginEventHandler.mock.calls[0][0].detail).toEqual({
|
||||
requestUrl: '/path/to/editing-sessions/?title=&type=some-type',
|
||||
});
|
||||
|
||||
// visual loading state should be active
|
||||
await Promise.resolve(); // trigger next rendering
|
||||
|
||||
expect(handleError).not.toHaveBeenCalled();
|
||||
expect(global.fetch).toHaveBeenCalledWith(
|
||||
'/path/to/editing-sessions/?title=&type=some-type',
|
||||
expect.any(Object),
|
||||
);
|
||||
|
||||
// simulate the request completing
|
||||
await Promise.resolve();
|
||||
|
||||
// should not update HTML just yet
|
||||
expect(
|
||||
document.getElementById('editing-sessions').querySelectorAll('li')
|
||||
.length,
|
||||
).toEqual(0);
|
||||
|
||||
await flushPromises();
|
||||
|
||||
// should still not update HTML just yet
|
||||
expect(
|
||||
document.getElementById('editing-sessions').querySelectorAll('li')
|
||||
.length,
|
||||
).toEqual(0);
|
||||
|
||||
// switch focus to a different element but still inside the target container
|
||||
document.getElementById('button2').focus();
|
||||
|
||||
await flushPromises();
|
||||
|
||||
// should still not update HTML just yet
|
||||
expect(
|
||||
document.getElementById('editing-sessions').querySelectorAll('li')
|
||||
.length,
|
||||
).toEqual(0);
|
||||
|
||||
// switch focus to an element outside the target container
|
||||
document.getElementById('submit').focus();
|
||||
|
||||
const successEvent = await onSuccess;
|
||||
|
||||
// should dispatch success event
|
||||
expect(successEvent.detail).toEqual({
|
||||
requestUrl: '/path/to/editing-sessions/?title=&type=some-type',
|
||||
results: expect.any(String),
|
||||
});
|
||||
|
||||
// should update HTML
|
||||
expect(
|
||||
document.getElementById('editing-sessions').querySelectorAll('li')
|
||||
.length,
|
||||
).toEqual(5);
|
||||
|
||||
await flushPromises();
|
||||
|
||||
// should NOT update the current URL
|
||||
// as the reflect-value attribute is not set
|
||||
expect(window.location.search).toEqual('');
|
||||
});
|
||||
|
||||
it('should not wait until the target loses focus if defer value is set to false', async () => {
|
||||
// unset the defer value (default is false)
|
||||
const form = document.querySelector('form');
|
||||
form.removeAttribute('data-w-swap-defer-value');
|
||||
|
||||
const onSuccess = new Promise((resolve) => {
|
||||
document.addEventListener('w-swap:success', resolve);
|
||||
});
|
||||
|
||||
const beginEventHandler = jest.fn();
|
||||
document.addEventListener('w-swap:begin', beginEventHandler);
|
||||
|
||||
fetch.mockResponseSuccessText(getMockResultsWithButtons(5));
|
||||
|
||||
expect(handleError).not.toHaveBeenCalled();
|
||||
expect(global.fetch).not.toHaveBeenCalled();
|
||||
|
||||
document.getElementById('submit').click();
|
||||
|
||||
// focus on an element inside the target container
|
||||
document.getElementById('button1').focus();
|
||||
|
||||
expect(beginEventHandler).not.toHaveBeenCalled();
|
||||
|
||||
jest.runAllTimers(); // submit is debounced
|
||||
|
||||
// should fire a begin event before the request is made
|
||||
expect(beginEventHandler).toHaveBeenCalledTimes(1);
|
||||
expect(beginEventHandler.mock.calls[0][0].detail).toEqual({
|
||||
requestUrl: '/path/to/editing-sessions/?title=&type=some-type',
|
||||
});
|
||||
|
||||
// visual loading state should be active
|
||||
await Promise.resolve(); // trigger next rendering
|
||||
|
||||
expect(handleError).not.toHaveBeenCalled();
|
||||
expect(global.fetch).toHaveBeenCalledWith(
|
||||
'/path/to/editing-sessions/?title=&type=some-type',
|
||||
expect.any(Object),
|
||||
);
|
||||
|
||||
// simulate the request completing
|
||||
await Promise.resolve();
|
||||
|
||||
// should not update HTML just yet
|
||||
expect(
|
||||
document.getElementById('editing-sessions').querySelectorAll('li')
|
||||
.length,
|
||||
).toEqual(0);
|
||||
|
||||
await flushPromises();
|
||||
|
||||
// if the deferred write happens (which should not), the test will
|
||||
// time out because the focus is still inside the target container
|
||||
const successEvent = await onSuccess;
|
||||
|
||||
// should dispatch success event
|
||||
expect(successEvent.detail).toEqual({
|
||||
requestUrl: '/path/to/editing-sessions/?title=&type=some-type',
|
||||
results: expect.any(String),
|
||||
});
|
||||
|
||||
// should update HTML
|
||||
expect(
|
||||
document.getElementById('editing-sessions').querySelectorAll('li')
|
||||
.length,
|
||||
).toEqual(5);
|
||||
|
||||
await flushPromises();
|
||||
|
||||
// should NOT update the current URL
|
||||
// as the reflect-value attribute is not set
|
||||
expect(window.location.search).toEqual('');
|
||||
});
|
||||
|
||||
it('should write immediately if there is a deferred write but we no longer need to defer', async () => {
|
||||
const successEvents = [];
|
||||
const onSuccess = new Promise((resolve) => {
|
||||
document.addEventListener('w-swap:success', (event) => {
|
||||
successEvents.push(event);
|
||||
resolve(event);
|
||||
});
|
||||
});
|
||||
|
||||
const beginEventHandler = jest.fn();
|
||||
document.addEventListener('w-swap:begin', beginEventHandler);
|
||||
|
||||
fetch.mockResponseSuccessText(getMockResultsWithButtons(5));
|
||||
|
||||
expect(handleError).not.toHaveBeenCalled();
|
||||
expect(global.fetch).not.toHaveBeenCalled();
|
||||
|
||||
document.getElementById('submit').click();
|
||||
|
||||
// focus on an element inside the target container
|
||||
document.getElementById('button1').focus();
|
||||
|
||||
expect(beginEventHandler).not.toHaveBeenCalled();
|
||||
|
||||
jest.runAllTimers(); // submit is debounced
|
||||
|
||||
// should fire a begin event before the request is made
|
||||
expect(beginEventHandler).toHaveBeenCalledTimes(1);
|
||||
expect(beginEventHandler.mock.calls[0][0].detail).toEqual({
|
||||
requestUrl: '/path/to/editing-sessions/?title=&type=some-type',
|
||||
});
|
||||
|
||||
// visual loading state should be active
|
||||
await Promise.resolve(); // trigger next rendering
|
||||
|
||||
expect(handleError).not.toHaveBeenCalled();
|
||||
expect(global.fetch).toHaveBeenCalledWith(
|
||||
'/path/to/editing-sessions/?title=&type=some-type',
|
||||
expect.any(Object),
|
||||
);
|
||||
|
||||
// simulate the request completing
|
||||
await Promise.resolve();
|
||||
|
||||
// should not update HTML just yet
|
||||
expect(
|
||||
document.getElementById('editing-sessions').querySelectorAll('li')
|
||||
.length,
|
||||
).toEqual(0);
|
||||
|
||||
await flushPromises();
|
||||
|
||||
// should still not update HTML just yet
|
||||
expect(
|
||||
document.getElementById('editing-sessions').querySelectorAll('li')
|
||||
.length,
|
||||
).toEqual(0);
|
||||
|
||||
// instead of switching the focus outside, we set the defer value
|
||||
// to false, so we can test the case where a new update is not deferred but
|
||||
// there is a deferred write and the focus is still inside the target container
|
||||
const form = document.querySelector('form');
|
||||
form.setAttribute('data-w-swap-defer-value', 'false');
|
||||
|
||||
// change the input so we can trigger a new request with a different URL to check
|
||||
document.querySelector('input[name="title"]').value = 'newvalue';
|
||||
|
||||
// submit the form again
|
||||
form.submit();
|
||||
fetch.mockResponseSuccessText(getMockResultsWithButtons(8));
|
||||
jest.runAllTimers();
|
||||
await flushPromises();
|
||||
|
||||
expect(global.fetch).toHaveBeenCalledWith(
|
||||
'/path/to/editing-sessions/?title=newvalue&type=some-type',
|
||||
expect.any(Object),
|
||||
);
|
||||
|
||||
const successEvent = await onSuccess;
|
||||
|
||||
// should dispatch success event
|
||||
expect(successEvent.detail).toEqual({
|
||||
requestUrl: '/path/to/editing-sessions/?title=newvalue&type=some-type',
|
||||
results: expect.any(String),
|
||||
});
|
||||
|
||||
// should skip the deferred write and instead write the last request's response (8 items)
|
||||
expect(
|
||||
document.getElementById('editing-sessions').querySelectorAll('li')
|
||||
.length,
|
||||
).toEqual(8);
|
||||
|
||||
await flushPromises();
|
||||
|
||||
// should still use the last request's response instead of running the deferred write
|
||||
expect(
|
||||
document.getElementById('editing-sessions').querySelectorAll('li')
|
||||
.length,
|
||||
).toEqual(8);
|
||||
|
||||
// Simulate triggering the unlikely edge case of the focusout event being triggered,
|
||||
// with no deferred writes left, which theoretically could only happen if the
|
||||
// defer value was changed to false at midpoint like in this test case.
|
||||
// We need to regain focus on an element in the target container first,
|
||||
// because the previous element that was focused is no longer in the DOM.
|
||||
document.getElementById('button1').focus();
|
||||
// then we focus out
|
||||
document.getElementById('submit').focus();
|
||||
|
||||
jest.runAllTimers();
|
||||
await flushPromises();
|
||||
|
||||
// should still use the last request's response instead of running the deferred write
|
||||
expect(
|
||||
document.getElementById('editing-sessions').querySelectorAll('li')
|
||||
.length,
|
||||
).toEqual(8);
|
||||
|
||||
// the success event should be dispatched only once as the deferred write was skipped
|
||||
expect(successEvents).toHaveLength(1);
|
||||
expect(successEvents[0].detail.requestUrl).toEqual(
|
||||
'/path/to/editing-sessions/?title=newvalue&type=some-type',
|
||||
);
|
||||
|
||||
// should NOT update the current URL
|
||||
// as the reflect-value attribute is not set
|
||||
expect(window.location.search).toEqual('');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -47,6 +47,7 @@ export class SwapController extends Controller<
|
|||
icon: { default: '', type: String },
|
||||
loading: { default: false, type: Boolean },
|
||||
reflect: { default: false, type: Boolean },
|
||||
defer: { default: false, type: Boolean },
|
||||
src: { default: '', type: String },
|
||||
jsonPath: { default: '', type: String },
|
||||
target: { default: '#listing-results', type: String },
|
||||
|
|
@ -62,6 +63,8 @@ export class SwapController extends Controller<
|
|||
declare iconValue: string;
|
||||
declare loadingValue: boolean;
|
||||
declare reflectValue: boolean;
|
||||
/** Defer writing the results until the focus has left the target container */
|
||||
declare deferValue: boolean;
|
||||
declare srcValue: string;
|
||||
declare jsonPathValue: string;
|
||||
declare targetValue: string;
|
||||
|
|
@ -77,6 +80,8 @@ export class SwapController extends Controller<
|
|||
searchLazy?: { (...args: any[]): void; cancel(): void };
|
||||
/** Debounced function to submit the serialised form and then replace the DOM */
|
||||
submitLazy?: { (...args: any[]): void; cancel(): void };
|
||||
/** A function that writes the HTML to the target */
|
||||
writeDeferred?: () => Promise<string>;
|
||||
|
||||
connect() {
|
||||
this.srcValue =
|
||||
|
|
@ -314,26 +319,69 @@ export class SwapController extends Controller<
|
|||
return response.text();
|
||||
})
|
||||
.then((results) => {
|
||||
target.innerHTML = results;
|
||||
const write = async () => {
|
||||
target.innerHTML = results;
|
||||
|
||||
if (this.reflectValue) {
|
||||
const event = this.dispatch('reflect', {
|
||||
cancelable: true,
|
||||
detail: { requestUrl },
|
||||
if (this.reflectValue) {
|
||||
const event = this.dispatch('reflect', {
|
||||
cancelable: true,
|
||||
detail: { requestUrl },
|
||||
target,
|
||||
});
|
||||
if (!event.defaultPrevented) {
|
||||
this.reflectParams(requestUrl);
|
||||
}
|
||||
}
|
||||
|
||||
this.dispatch('success', {
|
||||
cancelable: false,
|
||||
detail: { requestUrl, results },
|
||||
target,
|
||||
});
|
||||
if (!event.defaultPrevented) {
|
||||
this.reflectParams(requestUrl);
|
||||
}
|
||||
|
||||
return results;
|
||||
};
|
||||
|
||||
// If the currently focused element is within the target container,
|
||||
// defer the write until the focus has left the container.
|
||||
if (
|
||||
this.deferValue &&
|
||||
document.activeElement &&
|
||||
target.contains(document.activeElement)
|
||||
) {
|
||||
return new Promise((resolve, reject) => {
|
||||
this.writeDeferred = write;
|
||||
|
||||
const handleFocusOut = (event: FocusEvent) => {
|
||||
// If the new focus is still within the target container, do nothing
|
||||
if (target.contains(event.relatedTarget as Node | null)) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (this.writeDeferred) {
|
||||
this.writeDeferred().then(resolve).catch(reject);
|
||||
// Clear the deferred write to ensure it's not called again
|
||||
this.writeDeferred = undefined;
|
||||
} else {
|
||||
// The deferred write has been cleared but this listener is still
|
||||
// triggered, which is unlikely to happen but possible
|
||||
// (e.g. another request was made but the focus is still here and
|
||||
// deferValue is set to false), so just resolve
|
||||
resolve(results);
|
||||
}
|
||||
|
||||
// Done for the current data, remove the listener
|
||||
target.removeEventListener('focusout', handleFocusOut);
|
||||
};
|
||||
|
||||
target.addEventListener('focusout', handleFocusOut);
|
||||
});
|
||||
}
|
||||
|
||||
this.dispatch('success', {
|
||||
cancelable: false,
|
||||
detail: { requestUrl, results },
|
||||
target,
|
||||
});
|
||||
|
||||
return results;
|
||||
// If there's a previously deferred write, and we're not in a state that
|
||||
// needs a deferred write, clear the previous one and write immediately.
|
||||
this.writeDeferred = undefined;
|
||||
return write();
|
||||
})
|
||||
.catch((error) => {
|
||||
if (signal.aborted) return;
|
||||
|
|
|
|||
|
|
@ -28,6 +28,7 @@
|
|||
data-w-swap-target-value="#w-editing-sessions"
|
||||
data-w-swap-src-value="{{ ping_url }}"
|
||||
data-w-swap-json-path-value="html"
|
||||
data-w-swap-defer-value="true"
|
||||
data-w-action-continue-value="true"
|
||||
data-w-action-url-value="{{ release_url }}"
|
||||
data-w-session-w-dialog-outlet="[data-edit-form] [data-controller='w-dialog']#w-overwrite-changes-dialog"
|
||||
|
|
|
|||
Ładowanie…
Reference in New Issue