Skip to content

Commit 35d22e5

Browse files
zamooredchyundidoo
authored
PopoverPrimitive - Allow dynamic toggle element (#3189)
Co-authored-by: Dylan Hyun <[email protected]> Co-authored-by: Cristiano Rastelli <[email protected]>
1 parent 1451e49 commit 35d22e5

File tree

3 files changed

+138
-38
lines changed

3 files changed

+138
-38
lines changed

‎.changeset/brave-owls-jump.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"@hashicorp/design-system-components": minor
3+
---
4+
5+
<!-- START utilities/popover-primitive -->
6+
`PopoverPrimitive` - Added support for dynamic swap/injection of the toggle element.
7+
<!-- END -->

‎packages/components/src/components/hds/popover-primitive/index.ts‎

Lines changed: 54 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ export interface SetupPrimitivePopoverModifier {
6262
export default class HdsPopoverPrimitive extends Component<HdsPopoverPrimitiveSignature> {
6363
@tracked private _isOpen;
6464
@tracked private _isClosing = false;
65+
@tracked private _anchoredPositionOptions?: HdsAnchoredPositionOptions;
6566
private _containerElement?: HTMLElement;
6667
private _toggleElement?: HTMLButtonElement;
6768
private _popoverElement?: HTMLElement;
@@ -106,13 +107,23 @@ export default class HdsPopoverPrimitive extends Component<HdsPopoverPrimitiveSi
106107
);
107108

108109
setupPrimitiveToggle = modifier<SetupPrimitiveToggleModifier>(
109-
(element: HTMLButtonElement): void => {
110+
(element: HTMLButtonElement) => {
110111
this._toggleElement = element;
111112

112113
assert(
113114
`The toggle element of "Hds::PopoverPrimitive" must be a <button>; element received: <${element.tagName.toLowerCase()}>`,
114115
element instanceof HTMLButtonElement
115116
);
117+
118+
this._linkToggleAndPopover();
119+
120+
// Return a teardown function to clean up the modifier's side effects.
121+
// This is a safeguard against bugs where this element might be
122+
// cached and re-parented in the DOM, rather than being fully destroyed.
123+
return () => {
124+
element.removeAttribute('aria-controls');
125+
element.removeAttribute('popovertarget');
126+
};
116127
}
117128
);
118129

@@ -126,23 +137,8 @@ export default class HdsPopoverPrimitive extends Component<HdsPopoverPrimitiveSi
126137

127138
// We need to create a popoverId in order to connect the popover and the toggle with aria-controls
128139
// and an id is needed to implement `onclick` event listeners
129-
if (this._toggleElement) {
130-
let popoverId;
131-
if (this._popoverElement.id) {
132-
popoverId = this._popoverElement.id;
133-
} else {
134-
// we need a DOM id for the `aria-controls` and `popovertarget` attributes
135-
popoverId = guidFor(this);
136-
this._popoverElement.id = popoverId;
137-
}
138-
this._toggleElement.setAttribute('aria-controls', popoverId);
139-
140-
// for the click events we don't use `onclick` event listeners, but we rely on the `popovertarget` attribute
141-
// provided by the Popover API which does all the magic for us without needing JS code
142-
// (important: to work it needs to be applied to a button)
143-
if (this.enableClickEvents) {
144-
this._toggleElement.setAttribute('popovertarget', popoverId);
145-
}
140+
if (!this._popoverElement.id) {
141+
this._popoverElement.id = guidFor(this);
146142
}
147143

148144
// this should be an extremely edge case, but in the case the popover needs to be initially forced to be open
@@ -167,27 +163,58 @@ export default class HdsPopoverPrimitive extends Component<HdsPopoverPrimitiveSi
167163
registerEvent(this._popoverElement, ['toggle', this.onTogglePopover]);
168164

169165
// we need to spread the argument because if it's set via `{{ hash … }}` Ember complains when we overwrite one of its values
170-
const anchoredPositionOptions: HdsAnchoredPositionOptions = {
166+
this._anchoredPositionOptions = {
171167
...named.anchoredPositionOptions,
172168
};
173169

174-
// Apply the `hds-anchored-position` modifier to the "popover" element
175-
// (notice: this function runs the first time when the element the modifier was applied to is inserted into the DOM, and it autotracks while running.
176-
// Any tracked values that it accesses will be tracked, including the arguments it receives, and if any of them changes, the function will run again)
177-
// This modifiers uses the Floating UI library to provide:
178-
// - positioning of the "popover" in relation to the "toggle"
179-
// - collision detection (optional)
170+
this._linkToggleAndPopover();
171+
}
172+
);
173+
174+
// Apply the `hds-anchored-position` modifier to the "popover" element
175+
// (notice: this function runs the first time when the element the modifier was applied to is inserted into the DOM, and it autotracks while running.
176+
// Any tracked values that it accesses will be tracked, including the arguments it receives, and if any of them changes, the function will run again)
177+
// This modifiers uses the Floating UI library to provide:
178+
// - positioning of the "popover" in relation to the "toggle"
179+
// - collision detection (optional)
180+
private _applyAnchoredPositionModifier(): void {
181+
if (
182+
this._toggleElement !== undefined &&
183+
this._popoverElement !== undefined &&
184+
this._anchoredPositionOptions !== undefined
185+
) {
180186
// eslint-disable-next-line ember/no-runloop
181187
next((): void => {
182188
// @ts-expect-error: known issue with type of invocation
183189
anchoredPositionModifier(
184190
this._popoverElement, // element the modifier is attached to
185191
[this._toggleElement], // positional arguments
186-
anchoredPositionOptions // named arguments
192+
this._anchoredPositionOptions // named arguments
187193
);
188194
});
189195
}
190-
);
196+
}
197+
198+
private _linkToggleAndPopover(): void {
199+
if (
200+
this._toggleElement === undefined ||
201+
this._popoverElement === undefined
202+
) {
203+
return;
204+
}
205+
206+
const popoverId = this._popoverElement.id;
207+
208+
this._toggleElement.setAttribute('aria-controls', popoverId);
209+
210+
if (this.enableClickEvents) {
211+
this._toggleElement.setAttribute('popovertarget', popoverId);
212+
} else {
213+
this._toggleElement.removeAttribute('popovertarget');
214+
}
215+
216+
this._applyAnchoredPositionModifier();
217+
}
191218

192219
@action
193220
showPopover(): void {

‎showcase/tests/integration/components/hds/popover-primitive/index-test.js‎

Lines changed: 77 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ module(
2121
await render(hbs`
2222
<Hds::PopoverPrimitive @enableClickEvents={{true}} as |PP|>
2323
<div {{PP.setupPrimitiveContainer}}>
24-
<button {{PP.setupPrimitiveToggle}} />
24+
<button {{PP.setupPrimitiveToggle}} type="button" />
2525
<main {{PP.setupPrimitivePopover}} />
2626
</div>
2727
</Hds::PopoverPrimitive>
@@ -38,7 +38,7 @@ module(
3838
await render(hbs`
3939
<Hds::PopoverPrimitive @enableClickEvents={{true}} as |PP|>
4040
<div {{PP.setupPrimitiveContainer}}>
41-
<button {{PP.setupPrimitiveToggle}} id="test-popover-primitive-toggle" />
41+
<button {{PP.setupPrimitiveToggle}} id="test-popover-primitive-toggle" type="button" />
4242
<div {{PP.setupPrimitivePopover}} id="test-popover-primitive-content" />
4343
</div>
4444
</Hds::PopoverPrimitive>
@@ -57,7 +57,7 @@ module(
5757
await render(hbs`
5858
<Hds::PopoverPrimitive @enableSoftEvents={{true}} as |PP|>
5959
<div {{PP.setupPrimitiveContainer}}>
60-
<button {{PP.setupPrimitiveToggle}} id="test-popover-primitive-toggle" />
60+
<button {{PP.setupPrimitiveToggle}} id="test-popover-primitive-toggle" type="button" />
6161
<div {{PP.setupPrimitivePopover}} id="test-popover-primitive-content" />
6262
</div>
6363
</Hds::PopoverPrimitive>
@@ -82,7 +82,7 @@ module(
8282
await render(hbs`
8383
<Hds::PopoverPrimitive @enableClickEvents={{true}} as |PP|>
8484
<div {{PP.setupPrimitiveContainer}}>
85-
<button {{PP.setupPrimitiveToggle}} id="test-popover-primitive-toggle" />
85+
<button {{PP.setupPrimitiveToggle}} id="test-popover-primitive-toggle" type="button" />
8686
<div {{PP.setupPrimitivePopover}} id="test-popover-primitive-content" />
8787
</div>
8888
</Hds::PopoverPrimitive>
@@ -103,6 +103,72 @@ module(
103103
// should go back to hidden
104104
assert.dom('#test-popover-primitive-content').isNotVisible();
105105
});
106+
test('it should continue to work when the toggle element is dynamically swapped', async function (assert) {
107+
this.set('isSwapped', false);
108+
109+
await render(hbs`
110+
<Hds::PopoverPrimitive @enableClickEvents={{true}} as |PP|>
111+
<div {{PP.setupPrimitiveContainer}}>
112+
{{#if this.isSwapped}}
113+
<button data-test-id="replacement-toggle" type="button" {{PP.setupPrimitiveToggle}}>
114+
Replacement
115+
</button>
116+
{{else}}
117+
<button data-test-id="original-toggle" type="button" {{PP.setupPrimitiveToggle}}>
118+
Original
119+
</button>
120+
{{/if}}
121+
<div data-test-id="popover-content" {{PP.setupPrimitivePopover}}>
122+
Content
123+
</div>
124+
</div>
125+
</Hds::PopoverPrimitive>
126+
`);
127+
128+
// verify the initial toggle works as expected
129+
assert
130+
.dom('[data-test-id="original-toggle"]')
131+
.exists('The original toggle is rendered');
132+
assert
133+
.dom('[data-test-id="popover-content"]')
134+
.isNotVisible('The popover is initially hidden');
135+
136+
await click('[data-test-id="original-toggle"]');
137+
assert
138+
.dom('[data-test-id="popover-content"]')
139+
.isVisible('The popover becomes visible after the first click');
140+
141+
await click('[data-test-id="original-toggle"]');
142+
assert
143+
.dom('[data-test-id="popover-content"]')
144+
.isNotVisible('The popover is hidden again');
145+
146+
// swap the toggle element
147+
this.set('isSwapped', true);
148+
assert
149+
.dom('[data-test-id="original-toggle"]')
150+
.doesNotExist('The original toggle is removed');
151+
assert
152+
.dom('[data-test-id="replacement-toggle"]')
153+
.exists('The replacement toggle is rendered');
154+
155+
// verify the *new* toggle now controls the popover
156+
assert
157+
.dom('[data-test-id="popover-content"]')
158+
.isNotVisible('The popover remains hidden after the swap');
159+
160+
await click('[data-test-id="replacement-toggle"]');
161+
assert
162+
.dom('[data-test-id="popover-content"]')
163+
.isVisible(
164+
'The popover becomes visible when the new toggle is clicked',
165+
);
166+
167+
await click('[data-test-id="replacement-toggle"]');
168+
assert
169+
.dom('[data-test-id="popover-content"]')
170+
.isNotVisible('The popover is hidden again by the new toggle');
171+
});
106172
skip('it should toggle the popover visibility on click', async function (assert) {
107173
await render(hbs`
108174
<Hds::PopoverPrimitive @enableClickEvents={{true}}>
@@ -141,7 +207,7 @@ module(
141207
as |PP|
142208
>
143209
<div {{PP.setupPrimitiveContainer}}>
144-
<button {{PP.setupPrimitiveToggle}} id="test-popover-primitive-toggle" />
210+
<button {{PP.setupPrimitiveToggle}} id="test-popover-primitive-toggle" />
145211
<div {{PP.setupPrimitivePopover}} />
146212
</div>
147213
</Hds::PopoverPrimitive>
@@ -165,7 +231,7 @@ module(
165231
await render(hbs`
166232
<Hds::PopoverPrimitive as |PP|>
167233
<div {{PP.setupPrimitiveContainer}}>
168-
<button {{PP.setupPrimitiveToggle}} id="test-popover-primitive-toggle" />
234+
<button {{PP.setupPrimitiveToggle}} id="test-popover-primitive-toggle" type="button" />
169235
<div {{PP.setupPrimitivePopover}} />
170236
</div>
171237
</Hds::PopoverPrimitive>
@@ -178,7 +244,7 @@ module(
178244
await render(hbs`
179245
<Hds::PopoverPrimitive @enableClickEvents={{true}} as |PP|>
180246
<div {{PP.setupPrimitiveContainer}}>
181-
<button {{PP.setupPrimitiveToggle}} id="test-popover-primitive-toggle" />
247+
<button {{PP.setupPrimitiveToggle}} id="test-popover-primitive-toggle" type="button" />
182248
<div {{PP.setupPrimitivePopover}} id="test-popover-primitive-popover" />
183249
</div>
184250
</Hds::PopoverPrimitive>
@@ -191,7 +257,7 @@ module(
191257
await render(hbs`
192258
<Hds::PopoverPrimitive as |PP|>
193259
<div {{PP.setupPrimitiveContainer}}>
194-
<button {{PP.setupPrimitiveToggle}} id="test-popover-primitive-toggle" />
260+
<button {{PP.setupPrimitiveToggle}} id="test-popover-primitive-toggle" type="button" />
195261
<div {{PP.setupPrimitivePopover}} id="test-popover-primitive-content" />
196262
</div>
197263
</Hds::PopoverPrimitive>
@@ -204,7 +270,7 @@ module(
204270
await render(hbs`
205271
<Hds::PopoverPrimitive @enableClickEvents={{true}} @isOpen={{true}} as |PP|>
206272
<div {{PP.setupPrimitiveContainer}}>
207-
<button {{PP.setupPrimitiveToggle}} id="test-popover-primitive-toggle" />
273+
<button {{PP.setupPrimitiveToggle}} id="test-popover-primitive-toggle" type="button" />
208274
<div {{PP.setupPrimitivePopover}} id="test-popover-primitive-content" />
209275
</div>
210276
</Hds::PopoverPrimitive>
@@ -226,7 +292,7 @@ module(
226292
await render(hbs`
227293
<Hds::PopoverPrimitive @enableClickEvents={{true}} @isOpen={{true}} as |PP|>
228294
<div {{PP.setupPrimitiveContainer}}>
229-
<button {{PP.setupPrimitiveToggle}} id="test-popover-primitive-toggle" />
295+
<button {{PP.setupPrimitiveToggle}} id="test-popover-primitive-toggle" type="button" />
230296
<div {{PP.setupPrimitivePopover}} id="test-popover-primitive-content" />
231297
</div>
232298
</Hds::PopoverPrimitive>
@@ -244,7 +310,7 @@ module(
244310
await render(hbs`
245311
<Hds::PopoverPrimitive @enableClickEvents={{true}} @isOpen={{true}} as |PP|>
246312
<div {{PP.setupPrimitiveContainer}}>
247-
<button {{PP.setupPrimitiveToggle}} id="test-popover-primitive-toggle" />
313+
<button {{PP.setupPrimitiveToggle}} id="test-popover-primitive-toggle" type="button" />
248314
<div {{PP.setupPrimitivePopover}} id="test-popover-primitive-content" />
249315
</div>
250316
</Hds::PopoverPrimitive>

0 commit comments

Comments
 (0)