Skip to content

Commit

Permalink
Fix: solution to double selection (#958)
Browse files Browse the repository at this point in the history 
* solution to double selection

* write unit test for select

* add more tests
  • Loading branch information
@nielslyngsoe
nielslyngsoe authored Nov 19, 2024
1 parent c28c618 commit c305dfc
Show file tree
Hide file tree
Showing 5 changed files with 110 additions and 36 deletions.
20 changes: 13 additions & 7 deletions packages/uui-base/lib/mixins/SelectableMixin.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ export const SelectableMixin = <T extends Constructor<LitElement>>(
const oldVal = this._selectable;
this._selectable = newVal;
// Potentially problematic as a component might need focus for another feature when not selectable:
if (!this.selectableTarget) {
// If not selectable target, then make it self selectable. (A selectable target should be made focusable by the component itself)
if (this.selectableTarget === this) {
// If the selectable target, then make it self selectable. (A different selectable target should be made focusable by the component itself)
this.setAttribute('tabindex', `${newVal ? '0' : '-1'}`);
}
this.requestUpdate('selectable', oldVal);
Expand All @@ -80,10 +80,16 @@ export const SelectableMixin = <T extends Constructor<LitElement>>(
}

private handleSelectKeydown = (e: KeyboardEvent) => {
//if (e.composedPath().indexOf(this.selectableTarget) !== -1) {
if (this.selectableTarget === this) {
if (e.key !== ' ' && e.key !== 'Enter') return;
this._toggleSelect();
const composePath = e.composedPath();
if (
(this._selectable || (this.deselectable && this.selected)) &&
composePath.indexOf(this.selectableTarget) === 0
) {
if (this.selectableTarget === this) {
if (e.code !== 'Space' && e.code !== 'Enter') return;
this._toggleSelect();
e.preventDefault();
}
}
};

Expand Down Expand Up @@ -112,7 +118,7 @@ export const SelectableMixin = <T extends Constructor<LitElement>>(
}

private _toggleSelect() {
// Only allow for select-interaction if selectable is true. Deselectable is ignorered in this case, we do not want a DX where only deselection is a possibility..
// Only allow for select-interaction if selectable is true. Deselectable is ignored in this case, we do not want a DX where only deselection is a possibility..
if (!this.selectable) return;
if (this.deselectable === false) {
this._select();
Expand Down
31 changes: 24 additions & 7 deletions packages/uui-card-block-type/lib/uui-card-block-type.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ describe('UUICardBlockTypeElement', () => {
describe('events', () => {
describe('open', () => {
it('emits a open event when open-part is clicked', async () => {
const listener = oneEvent(element, UUICardEvent.OPEN, false);
const listener = oneEvent(element, UUICardEvent.OPEN);
const infoElement: HTMLElement | null =
element.shadowRoot!.querySelector('#open-part');
infoElement?.click();
Expand All @@ -93,25 +93,42 @@ describe('UUICardBlockTypeElement', () => {
it('emits a selected event when selectable', async () => {
element.selectable = true;
await elementUpdated(element);
const listener = oneEvent(element, UUISelectableEvent.SELECTED, false);
const listener = oneEvent(element, UUISelectableEvent.SELECTED);
element.click();
const event = await listener;
expect(event).to.exist;
expect(event.type).to.equal(UUISelectableEvent.SELECTED);
expect(element.selected).to.be.true;
});

it('can be selected with keyboard', async () => {
element.selectable = true;
await elementUpdated(element);
const listener = oneEvent(element, UUISelectableEvent.SELECTED);
element.dispatchEvent(new KeyboardEvent('keydown', { code: 'Enter' }));
const event = await listener;
expect(event).to.exist;
expect(event.type).to.equal(UUISelectableEvent.SELECTED);
expect(element.selected).to.be.true;

const unselectedListener = oneEvent(
element,
UUISelectableEvent.DESELECTED,
);
element.dispatchEvent(new KeyboardEvent('keydown', { code: 'Space' }));
const event2 = await unselectedListener;
expect(event2).to.exist;
expect(event2.type).to.equal(UUISelectableEvent.DESELECTED);
expect(element.selected).to.be.false;
});
});

describe('deselect', () => {
it('emits a deselected event when preselected', async () => {
element.selectable = true;
element.selected = true;
await elementUpdated(element);
const listener = oneEvent(
element,
UUISelectableEvent.DESELECTED,
false,
);
const listener = oneEvent(element, UUISelectableEvent.DESELECTED);
element.click();
const event = await listener;
expect(event).to.exist;
Expand Down
33 changes: 25 additions & 8 deletions packages/uui-card-content-node/lib/uui-card-content-node.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ describe('UUICardContentNodeElement', () => {
describe('events', () => {
describe('open', () => {
it('emits a open event when info is clicked', async () => {
const listener = oneEvent(element, UUICardEvent.OPEN, false);
const listener = oneEvent(element, UUICardEvent.OPEN);
const infoElement: HTMLElement | null =
element.shadowRoot!.querySelector('#open-part');
infoElement?.click();
Expand All @@ -85,7 +85,7 @@ describe('UUICardContentNodeElement', () => {
});

it('emits a open event when icon is clicked', async () => {
const listener = oneEvent(element, UUICardEvent.OPEN, false);
const listener = oneEvent(element, UUICardEvent.OPEN);
const iconElement: HTMLElement | null =
element.shadowRoot!.querySelector('#icon');
iconElement?.click();
Expand All @@ -99,25 +99,42 @@ describe('UUICardContentNodeElement', () => {
it('emits a selected event when selectable', async () => {
element.selectable = true;
await elementUpdated(element);
const listener = oneEvent(element, UUISelectableEvent.SELECTED, false);
const listener = oneEvent(element, UUISelectableEvent.SELECTED);
element.click();
const event = await listener;
expect(event).to.exist;
expect(event.type).to.equal(UUISelectableEvent.SELECTED);
expect(element.selected).to.be.true;
});

it('can be selected with keyboard', async () => {
element.selectable = true;
await elementUpdated(element);
const listener = oneEvent(element, UUISelectableEvent.SELECTED);
element.dispatchEvent(new KeyboardEvent('keydown', { code: 'Enter' }));
const event = await listener;
expect(event).to.exist;
expect(event.type).to.equal(UUISelectableEvent.SELECTED);
expect(element.selected).to.be.true;

const unselectedListener = oneEvent(
element,
UUISelectableEvent.DESELECTED,
);
element.dispatchEvent(new KeyboardEvent('keydown', { code: 'Space' }));
const event2 = await unselectedListener;
expect(event2).to.exist;
expect(event2.type).to.equal(UUISelectableEvent.DESELECTED);
expect(element.selected).to.be.false;
});
});

describe('deselect', () => {
it('emits a deselected event when preselected', async () => {
element.selectable = true;
element.selected = true;
await elementUpdated(element);
const listener = oneEvent(
element,
UUISelectableEvent.DESELECTED,
false,
);
const listener = oneEvent(element, UUISelectableEvent.DESELECTED);
element.click();
const event = await listener;
expect(event).to.exist;
Expand Down
30 changes: 23 additions & 7 deletions packages/uui-card-media/lib/uui-card-media.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ describe('UUICardMediaElement', () => {
describe('events', () => {
describe('open', () => {
it('emits a open event when open-part is clicked', async () => {
const listener = oneEvent(element, UUICardEvent.OPEN, false);
const listener = oneEvent(element, UUICardEvent.OPEN);
const infoElement: HTMLElement | null =
element.shadowRoot!.querySelector('#open-part');
infoElement?.click();
Expand All @@ -92,25 +92,41 @@ describe('UUICardMediaElement', () => {
it('emits a selected event when selectable', async () => {
element.selectable = true;
await elementUpdated(element);
const listener = oneEvent(element, UUISelectableEvent.SELECTED, false);
const listener = oneEvent(element, UUISelectableEvent.SELECTED);
element.click();
const event = await listener;
expect(event).to.exist;
expect(event.type).to.equal(UUISelectableEvent.SELECTED);
expect(element.selected).to.be.true;
});
it('can be selected with keyboard', async () => {
element.selectable = true;
await elementUpdated(element);
const listener = oneEvent(element, UUISelectableEvent.SELECTED);
element.dispatchEvent(new KeyboardEvent('keydown', { code: 'Enter' }));
const event = await listener;
expect(event).to.exist;
expect(event.type).to.equal(UUISelectableEvent.SELECTED);
expect(element.selected).to.be.true;

const unselectedListener = oneEvent(
element,
UUISelectableEvent.DESELECTED,
);
element.dispatchEvent(new KeyboardEvent('keydown', { code: 'Space' }));
const event2 = await unselectedListener;
expect(event2).to.exist;
expect(event2.type).to.equal(UUISelectableEvent.DESELECTED);
expect(element.selected).to.be.false;
});
});

describe('deselect', () => {
it('emits a deselected event when preselected', async () => {
element.selectable = true;
element.selected = true;
await elementUpdated(element);
const listener = oneEvent(
element,
UUISelectableEvent.DESELECTED,
false,
);
const listener = oneEvent(element, UUISelectableEvent.DESELECTED);
element.click();
const event = await listener;
expect(event).to.exist;
Expand Down
32 changes: 25 additions & 7 deletions packages/uui-card-user/lib/uui-card-user.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ describe('UUICardUserElement', () => {
describe('events', () => {
describe('open', () => {
it('emits a open event when open-part is clicked', async () => {
const listener = oneEvent(element, UUICardEvent.OPEN, false);
const listener = oneEvent(element, UUICardEvent.OPEN);
const infoElement: HTMLElement | null =
element.shadowRoot!.querySelector('#open-part');
infoElement?.click();
Expand All @@ -91,25 +91,43 @@ describe('UUICardUserElement', () => {
it('emits a selected event when selectable', async () => {
element.selectable = true;
await elementUpdated(element);
const listener = oneEvent(element, UUISelectableEvent.SELECTED, false);
const listener = oneEvent(element, UUISelectableEvent.SELECTED);
element.click();
const event = await listener;
expect(event).to.exist;
expect(event.type).to.equal(UUISelectableEvent.SELECTED);
expect(element.selected).to.be.true;
});

it('can be selected with keyboard', async () => {
element.selectable = true;
await elementUpdated(element);

const listener = oneEvent(element, UUISelectableEvent.SELECTED);
element.dispatchEvent(new KeyboardEvent('keydown', { code: 'Enter' }));
const event = await listener;
expect(event).to.exist;
expect(event.type).to.equal(UUISelectableEvent.SELECTED);
expect(element.selected).to.be.true;

const unselectedListener = oneEvent(
element,
UUISelectableEvent.DESELECTED,
);
element.dispatchEvent(new KeyboardEvent('keydown', { code: 'Space' }));
const event2 = await unselectedListener;
expect(event2).to.exist;
expect(event2.type).to.equal(UUISelectableEvent.DESELECTED);
expect(element.selected).to.be.false;
});
});

describe('deselect', () => {
it('emits a deselected event when preselected', async () => {
element.selectable = true;
element.selected = true;
await elementUpdated(element);
const listener = oneEvent(
element,
UUISelectableEvent.DESELECTED,
false,
);
const listener = oneEvent(element, UUISelectableEvent.DESELECTED);
element.click();
const event = await listener;
expect(event).to.exist;
Expand Down

0 comments on commit c305dfc

Please sign in to comment.