Skip to content

Commit

Permalink
fix(grid): grid focusgroup fix on mutationObserver (#3684)
Browse files Browse the repository at this point in the history
* fix(grid): grid focusgroup fix on mutationObserver

* fix(tags): fixed focusGroup controller

* refactor(reactive-controllers): update attach and post clear cache update timings

---------

Co-authored-by: Rajdeep Chandra <rajdeepc@adobe.com>
Co-authored-by: Piyush Vashisht <piyush17303@iiitd.ac.in>
Co-authored-by: Westbrook Johnson <westbrook.johnson@gmail.com>
  • Loading branch information
4 people authored Oct 5, 2023
1 parent a5dfada commit 5d47db5
Show file tree
Hide file tree
Showing 3 changed files with 100 additions and 13 deletions.
75 changes: 72 additions & 3 deletions packages/tags/test/tags.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ import {
pageUpEvent,
testForLitDevWarnings,
} from '../../../test/testing-helpers.js';
import { executeServerCommand } from '@web/test-runner-commands';
import { sendKeys } from '@web/test-runner-commands';
import { nextFrame } from '@spectrum-web-components/overlay/src/AbstractOverlay.js';

describe('Tags', () => {
testForLitDevWarnings(
Expand Down Expand Up @@ -176,6 +177,74 @@ describe('Tags', () => {

tag1.blur();
});

it('handles focus when Tag is deleted', async () => {
const el = await fixture<Tags>(
html`
<sp-tags>
<sp-tag deletable id="t1">Tag 1</sp-tag>
<sp-tag deletable id="t2">Tag 2</sp-tag>
<sp-tag deletable id="t3">Tag 3</sp-tag>
<sp-tag deletable id="t4">Tag 4</sp-tag>
<sp-tag deletable id="t5">Tag 5</sp-tag>
</sp-tags>
`
);

await elementUpdated(el);

const tag1 = el.querySelector('sp-tag#t1') as Tag;
const tag2 = el.querySelector('sp-tag#t2') as Tag;
const tag3 = el.querySelector('sp-tag#t3') as Tag;
const tag4 = el.querySelector('sp-tag#t4') as Tag;
const tag5 = el.querySelector('sp-tag#t5') as Tag;

tag1.focus();
await elementUpdated(el);

await sendKeys({
press: 'ArrowRight',
});
await elementUpdated(el);
await nextFrame();
await nextFrame();

expect(document.activeElement === tag2).to.be.true;

await sendKeys({
press: 'Delete',
});
await elementUpdated(el);
await nextFrame();
await nextFrame();

expect(document.activeElement === tag3).to.be.true;

await sendKeys({
press: 'ArrowRight',
});
await elementUpdated(el);
await nextFrame();
await nextFrame();
await sendKeys({
press: 'ArrowRight',
});
await elementUpdated(el);
await nextFrame();
await nextFrame();

expect(document.activeElement === tag5).to.be.true;

await sendKeys({
press: 'Delete',
});
await elementUpdated(el);
await nextFrame();
await nextFrame();

expect(document.activeElement === tag4).to.be.true;
});

it('will not focus [disabled] children', async () => {
const el = await fixture<Tags>(
html`
Expand Down Expand Up @@ -288,12 +357,12 @@ describe('Tags', () => {
const tagA = tagset2.querySelector('sp-tag:nth-child(1)') as Tag;
const tagB = tagset2.querySelector('sp-tag:nth-child(2)') as Tag;

await executeServerCommand('send-keys', {
await sendKeys({
press: 'Tab',
});
expect(document.activeElement === tag1).to.be.true;

await executeServerCommand('send-keys', {
await sendKeys({
press: 'Tab',
});
expect(document.activeElement === tagA).to.be.true;
Expand Down
35 changes: 26 additions & 9 deletions tools/reactive-controllers/src/FocusGroup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ OF ANY KIND, either express or implied. See the License for the specific languag
governing permissions and limitations under the License.
*/
import type { ReactiveController, ReactiveElement } from 'lit';
import { MutationController } from '@lit-labs/observers/mutation-controller.js';

type DirectionTypes = 'horizontal' | 'vertical' | 'both' | 'grid';
export type FocusGroupConfig<T> = {
Expand Down Expand Up @@ -39,6 +38,7 @@ export class FocusGroupController<T extends HTMLElement>
implements ReactiveController
{
protected cachedElements?: T[];
private mutationObserver: MutationObserver;

get currentIndex(): number {
if (this._currentIndex === -1) {
Expand Down Expand Up @@ -113,6 +113,8 @@ export class FocusGroupController<T extends HTMLElement>
// and the first rendered element.
offset = 0;

recentlyConnected = false;

constructor(
host: ReactiveElement,
{
Expand All @@ -124,14 +126,8 @@ export class FocusGroupController<T extends HTMLElement>
listenerScope,
}: FocusGroupConfig<T> = { elements: () => [] }
) {
new MutationController(host, {
config: {
childList: true,
subtree: true,
},
callback: () => {
this.handleItemMutation();
},
this.mutationObserver = new MutationObserver(() => {
this.handleItemMutation();
});
this.host = host;
this.host.addController(this);
Expand Down Expand Up @@ -196,8 +192,16 @@ export class FocusGroupController<T extends HTMLElement>
}

clearElementCache(offset = 0): void {
this.mutationObserver.disconnect();
delete this.cachedElements;
this.offset = offset;
requestAnimationFrame(() => {
this.elements.forEach((element) => {
this.mutationObserver.observe(element, {
attributes: true,
});
});
});
}

setCurrentIndexCircularly(diff: number): void {
Expand Down Expand Up @@ -333,10 +337,23 @@ export class FocusGroupController<T extends HTMLElement>
}

hostConnected(): void {
this.recentlyConnected = true;
this.addEventListeners();
}

hostDisconnected(): void {
this.mutationObserver.disconnect();
this.removeEventListeners();
}

hostUpdated(): void {
if (this.recentlyConnected) {
this.recentlyConnected = false;
this.elements.forEach((element) => {
this.mutationObserver.observe(element, {
attributes: true,
});
});
}
}
}
3 changes: 2 additions & 1 deletion tools/reactive-controllers/src/RovingTabindex.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ export class RovingTabindexController<
super.unmanage();
}

hostUpdated(): void {
override hostUpdated(): void {
super.hostUpdated();
if (!this.host.hasUpdated) {
this.manageTabindexes();
}
Expand Down

0 comments on commit 5d47db5

Please sign in to comment.