Skip to content

Commit c305dfc

Browse files
nielslyngsoenielslyngsoe
authored
Fix: solution to double selection (#958)
* solution to double selection * write unit test for select * add more tests
1 parent c28c618 commit c305dfc

File tree

5 files changed

+110
-36
lines changed

5 files changed

+110
-36
lines changed

‎packages/uui-base/lib/mixins/SelectableMixin.ts‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,8 @@ export const SelectableMixin = <T extends Constructor<LitElement>>(
5454
const oldVal = this._selectable;
5555
this._selectable = newVal;
5656
// Potentially problematic as a component might need focus for another feature when not selectable:
57-
if (!this.selectableTarget) {
58-
// If not selectable target, then make it self selectable. (A selectable target should be made focusable by the component itself)
57+
if (this.selectableTarget === this) {
58+
// If the selectable target, then make it self selectable. (A different selectable target should be made focusable by the component itself)
5959
this.setAttribute('tabindex', `${newVal ? '0' : '-1'}`);
6060
}
6161
this.requestUpdate('selectable', oldVal);
@@ -80,10 +80,16 @@ export const SelectableMixin = <T extends Constructor<LitElement>>(
8080
}
8181

8282
private handleSelectKeydown = (e: KeyboardEvent) => {
83-
//if (e.composedPath().indexOf(this.selectableTarget) !== -1) {
84-
if (this.selectableTarget === this) {
85-
if (e.key !== ' ' && e.key !== 'Enter') return;
86-
this._toggleSelect();
83+
const composePath = e.composedPath();
84+
if (
85+
(this._selectable || (this.deselectable && this.selected)) &&
86+
composePath.indexOf(this.selectableTarget) === 0
87+
) {
88+
if (this.selectableTarget === this) {
89+
if (e.code !== 'Space' && e.code !== 'Enter') return;
90+
this._toggleSelect();
91+
e.preventDefault();
92+
}
8793
}
8894
};
8995

@@ -112,7 +118,7 @@ export const SelectableMixin = <T extends Constructor<LitElement>>(
112118
}
113119

114120
private _toggleSelect() {
115-
// 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..
121+
// 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..
116122
if (!this.selectable) return;
117123
if (this.deselectable === false) {
118124
this._select();

‎packages/uui-card-block-type/lib/uui-card-block-type.test.ts‎

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ describe('UUICardBlockTypeElement', () => {
7979
describe('events', () => {
8080
describe('open', () => {
8181
it('emits a open event when open-part is clicked', async () => {
82-
const listener = oneEvent(element, UUICardEvent.OPEN, false);
82+
const listener = oneEvent(element, UUICardEvent.OPEN);
8383
const infoElement: HTMLElement | null =
8484
element.shadowRoot!.querySelector('#open-part');
8585
infoElement?.click();
@@ -93,25 +93,42 @@ describe('UUICardBlockTypeElement', () => {
9393
it('emits a selected event when selectable', async () => {
9494
element.selectable = true;
9595
await elementUpdated(element);
96-
const listener = oneEvent(element, UUISelectableEvent.SELECTED, false);
96+
const listener = oneEvent(element, UUISelectableEvent.SELECTED);
9797
element.click();
9898
const event = await listener;
9999
expect(event).to.exist;
100100
expect(event.type).to.equal(UUISelectableEvent.SELECTED);
101101
expect(element.selected).to.be.true;
102102
});
103+
104+
it('can be selected with keyboard', async () => {
105+
element.selectable = true;
106+
await elementUpdated(element);
107+
const listener = oneEvent(element, UUISelectableEvent.SELECTED);
108+
element.dispatchEvent(new KeyboardEvent('keydown', { code: 'Enter' }));
109+
const event = await listener;
110+
expect(event).to.exist;
111+
expect(event.type).to.equal(UUISelectableEvent.SELECTED);
112+
expect(element.selected).to.be.true;
113+
114+
const unselectedListener = oneEvent(
115+
element,
116+
UUISelectableEvent.DESELECTED,
117+
);
118+
element.dispatchEvent(new KeyboardEvent('keydown', { code: 'Space' }));
119+
const event2 = await unselectedListener;
120+
expect(event2).to.exist;
121+
expect(event2.type).to.equal(UUISelectableEvent.DESELECTED);
122+
expect(element.selected).to.be.false;
123+
});
103124
});
104125

105126
describe('deselect', () => {
106127
it('emits a deselected event when preselected', async () => {
107128
element.selectable = true;
108129
element.selected = true;
109130
await elementUpdated(element);
110-
const listener = oneEvent(
111-
element,
112-
UUISelectableEvent.DESELECTED,
113-
false,
114-
);
131+
const listener = oneEvent(element, UUISelectableEvent.DESELECTED);
115132
element.click();
116133
const event = await listener;
117134
expect(event).to.exist;

‎packages/uui-card-content-node/lib/uui-card-content-node.test.ts‎

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ describe('UUICardContentNodeElement', () => {
7575
describe('events', () => {
7676
describe('open', () => {
7777
it('emits a open event when info is clicked', async () => {
78-
const listener = oneEvent(element, UUICardEvent.OPEN, false);
78+
const listener = oneEvent(element, UUICardEvent.OPEN);
7979
const infoElement: HTMLElement | null =
8080
element.shadowRoot!.querySelector('#open-part');
8181
infoElement?.click();
@@ -85,7 +85,7 @@ describe('UUICardContentNodeElement', () => {
8585
});
8686

8787
it('emits a open event when icon is clicked', async () => {
88-
const listener = oneEvent(element, UUICardEvent.OPEN, false);
88+
const listener = oneEvent(element, UUICardEvent.OPEN);
8989
const iconElement: HTMLElement | null =
9090
element.shadowRoot!.querySelector('#icon');
9191
iconElement?.click();
@@ -99,25 +99,42 @@ describe('UUICardContentNodeElement', () => {
9999
it('emits a selected event when selectable', async () => {
100100
element.selectable = true;
101101
await elementUpdated(element);
102-
const listener = oneEvent(element, UUISelectableEvent.SELECTED, false);
102+
const listener = oneEvent(element, UUISelectableEvent.SELECTED);
103103
element.click();
104104
const event = await listener;
105105
expect(event).to.exist;
106106
expect(event.type).to.equal(UUISelectableEvent.SELECTED);
107107
expect(element.selected).to.be.true;
108108
});
109+
110+
it('can be selected with keyboard', async () => {
111+
element.selectable = true;
112+
await elementUpdated(element);
113+
const listener = oneEvent(element, UUISelectableEvent.SELECTED);
114+
element.dispatchEvent(new KeyboardEvent('keydown', { code: 'Enter' }));
115+
const event = await listener;
116+
expect(event).to.exist;
117+
expect(event.type).to.equal(UUISelectableEvent.SELECTED);
118+
expect(element.selected).to.be.true;
119+
120+
const unselectedListener = oneEvent(
121+
element,
122+
UUISelectableEvent.DESELECTED,
123+
);
124+
element.dispatchEvent(new KeyboardEvent('keydown', { code: 'Space' }));
125+
const event2 = await unselectedListener;
126+
expect(event2).to.exist;
127+
expect(event2.type).to.equal(UUISelectableEvent.DESELECTED);
128+
expect(element.selected).to.be.false;
129+
});
109130
});
110131

111132
describe('deselect', () => {
112133
it('emits a deselected event when preselected', async () => {
113134
element.selectable = true;
114135
element.selected = true;
115136
await elementUpdated(element);
116-
const listener = oneEvent(
117-
element,
118-
UUISelectableEvent.DESELECTED,
119-
false,
120-
);
137+
const listener = oneEvent(element, UUISelectableEvent.DESELECTED);
121138
element.click();
122139
const event = await listener;
123140
expect(event).to.exist;

‎packages/uui-card-media/lib/uui-card-media.test.ts‎

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ describe('UUICardMediaElement', () => {
7878
describe('events', () => {
7979
describe('open', () => {
8080
it('emits a open event when open-part is clicked', async () => {
81-
const listener = oneEvent(element, UUICardEvent.OPEN, false);
81+
const listener = oneEvent(element, UUICardEvent.OPEN);
8282
const infoElement: HTMLElement | null =
8383
element.shadowRoot!.querySelector('#open-part');
8484
infoElement?.click();
@@ -92,25 +92,41 @@ describe('UUICardMediaElement', () => {
9292
it('emits a selected event when selectable', async () => {
9393
element.selectable = true;
9494
await elementUpdated(element);
95-
const listener = oneEvent(element, UUISelectableEvent.SELECTED, false);
95+
const listener = oneEvent(element, UUISelectableEvent.SELECTED);
9696
element.click();
9797
const event = await listener;
9898
expect(event).to.exist;
9999
expect(event.type).to.equal(UUISelectableEvent.SELECTED);
100100
expect(element.selected).to.be.true;
101101
});
102+
it('can be selected with keyboard', async () => {
103+
element.selectable = true;
104+
await elementUpdated(element);
105+
const listener = oneEvent(element, UUISelectableEvent.SELECTED);
106+
element.dispatchEvent(new KeyboardEvent('keydown', { code: 'Enter' }));
107+
const event = await listener;
108+
expect(event).to.exist;
109+
expect(event.type).to.equal(UUISelectableEvent.SELECTED);
110+
expect(element.selected).to.be.true;
111+
112+
const unselectedListener = oneEvent(
113+
element,
114+
UUISelectableEvent.DESELECTED,
115+
);
116+
element.dispatchEvent(new KeyboardEvent('keydown', { code: 'Space' }));
117+
const event2 = await unselectedListener;
118+
expect(event2).to.exist;
119+
expect(event2.type).to.equal(UUISelectableEvent.DESELECTED);
120+
expect(element.selected).to.be.false;
121+
});
102122
});
103123

104124
describe('deselect', () => {
105125
it('emits a deselected event when preselected', async () => {
106126
element.selectable = true;
107127
element.selected = true;
108128
await elementUpdated(element);
109-
const listener = oneEvent(
110-
element,
111-
UUISelectableEvent.DESELECTED,
112-
false,
113-
);
129+
const listener = oneEvent(element, UUISelectableEvent.DESELECTED);
114130
element.click();
115131
const event = await listener;
116132
expect(event).to.exist;

‎packages/uui-card-user/lib/uui-card-user.test.ts‎

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ describe('UUICardUserElement', () => {
7777
describe('events', () => {
7878
describe('open', () => {
7979
it('emits a open event when open-part is clicked', async () => {
80-
const listener = oneEvent(element, UUICardEvent.OPEN, false);
80+
const listener = oneEvent(element, UUICardEvent.OPEN);
8181
const infoElement: HTMLElement | null =
8282
element.shadowRoot!.querySelector('#open-part');
8383
infoElement?.click();
@@ -91,25 +91,43 @@ describe('UUICardUserElement', () => {
9191
it('emits a selected event when selectable', async () => {
9292
element.selectable = true;
9393
await elementUpdated(element);
94-
const listener = oneEvent(element, UUISelectableEvent.SELECTED, false);
94+
const listener = oneEvent(element, UUISelectableEvent.SELECTED);
9595
element.click();
9696
const event = await listener;
9797
expect(event).to.exist;
9898
expect(event.type).to.equal(UUISelectableEvent.SELECTED);
9999
expect(element.selected).to.be.true;
100100
});
101+
102+
it('can be selected with keyboard', async () => {
103+
element.selectable = true;
104+
await elementUpdated(element);
105+
106+
const listener = oneEvent(element, UUISelectableEvent.SELECTED);
107+
element.dispatchEvent(new KeyboardEvent('keydown', { code: 'Enter' }));
108+
const event = await listener;
109+
expect(event).to.exist;
110+
expect(event.type).to.equal(UUISelectableEvent.SELECTED);
111+
expect(element.selected).to.be.true;
112+
113+
const unselectedListener = oneEvent(
114+
element,
115+
UUISelectableEvent.DESELECTED,
116+
);
117+
element.dispatchEvent(new KeyboardEvent('keydown', { code: 'Space' }));
118+
const event2 = await unselectedListener;
119+
expect(event2).to.exist;
120+
expect(event2.type).to.equal(UUISelectableEvent.DESELECTED);
121+
expect(element.selected).to.be.false;
122+
});
101123
});
102124

103125
describe('deselect', () => {
104126
it('emits a deselected event when preselected', async () => {
105127
element.selectable = true;
106128
element.selected = true;
107129
await elementUpdated(element);
108-
const listener = oneEvent(
109-
element,
110-
UUISelectableEvent.DESELECTED,
111-
false,
112-
);
130+
const listener = oneEvent(element, UUISelectableEvent.DESELECTED);
113131
element.click();
114132
const event = await listener;
115133
expect(event).to.exist;

0 commit comments

Comments
 (0)