Skip to content

Commit 75e8d07

Browse files
author
Helen Le
committed
Revert "fix(overlay): Programmatic clicks on elements outside of the overlay now registers (#5670)"
This reverts commit 14486d6.
1 parent 12b7382 commit 75e8d07

File tree

3 files changed

+11
-94
lines changed

3 files changed

+11
-94
lines changed

packages/overlay/README.md

Lines changed: 10 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ Some Overlays will always be passed focus (e.g. modal or page Overlays). When th
269269

270270
The `trigger` option accepts an `HTMLElement` or a `VirtualTrigger` from which to position the Overlay.
271271

272-
- You can import the `VirtualTrigger` class from the overlay package to create a virtual trigger that can be used to position an Overlay. This is useful when you want to position an Overlay relative to a point on the screen that is not an element in the DOM, like the mouse cursor.
272+
- You can import the `VirtualTrigger` class from the overlay package to create a virtual trigger that can be used to position an Overlay. This is useful when you want to position an Overlay relative to a point on the screen that is not an element in the DOM, like the mouse cursor.
273273

274274
The `type` of an Overlay outlines a number of things about the interaction model within which it works:
275275

@@ -408,8 +408,8 @@ The `overlay` value in this case will hold a reference to the actual `<sp-overla
408408

409409
"Fully" in this context means that all CSS transitions that have dispatched `transitionrun` events on the direct children of the `<sp-overlay>` element have successfully dispatched their `transitionend` or `transitioncancel` event. Keep in mind the following:
410410

411-
- `transition*` events bubble; this means that while transition events on light DOM content of those direct children will be heard, those events will not be taken into account
412-
- `transition*` events are not composed; this means that transition events on shadow DOM content of the direct children will not propagate to a level in the DOM where they can be heard
411+
- `transition*` events bubble; this means that while transition events on light DOM content of those direct children will be heard, those events will not be taken into account
412+
- `transition*` events are not composed; this means that transition events on shadow DOM content of the direct children will not propagate to a level in the DOM where they can be heard
413413

414414
This means that in both cases, if the transition is meant to be a part of the opening or closing of the overlay in question you will need to re-dispatch the `transitionrun`, `transitionend`, and `transitioncancel` events from that transition from the closest direct child of the `<sp-overlay>`.
415415

@@ -532,7 +532,6 @@ This means that in both cases, if the transition is meant to be a part of the op
532532
.triggerElement=${HTMLElement}
533533
.triggerInteraction=${'click' | 'longpress' | 'hover'}
534534
type=${'auto' | 'hint' | 'manual' | 'modal' | 'page'}
535-
?allow-outside-click=${boolean}
536535
></sp-overlay>
537536
```
538537

@@ -574,23 +573,6 @@ Common in `modal`/`page` overlays for full-screen content</sp-table-cell>
574573
</sp-table-body>
575574
</sp-table>
576575

577-
##### Deprecated Properties
578-
579-
> **⚠️ Deprecation Notice**: The `allow-outside-click` property is deprecated and will be removed in a future version.
580-
581-
The `allow-outside-click` property allows clicks outside the overlay to close it. **We do not recommend using this property for accessibility reasons** as it can cause unexpected behavior and accessibility issues. When set to `true`, it configures the focus trap to allow outside clicks, which may interfere with proper focus management and user expectations.
582-
583-
```html
584-
<!-- @deprecated Not recommended for accessibility reasons -->
585-
<sp-overlay trigger="trigger@click" allow-outside-click="true">
586-
<sp-popover>
587-
<p>This overlay can be closed by clicking outside</p>
588-
</sp-popover>
589-
</sp-overlay>
590-
```
591-
592-
**Alternative approaches**: Instead of using `allow-outside-click`, consider implementing explicit close buttons or using the `type="modal"` or `type="page"` overlay types which provide better accessibility and user experience.
593-
594576
#### Styling
595577

596578
`<sp-overlay>` element will use the `<dialog>` element or `popover` attribute to project your content onto the top-layer of the browser, without being moved in the DOM tree. That means that you can style your overlay content with whatever techniques you are already leveraging to style the content that doesn't get overlaid. This means standard CSS selectors, CSS Custom Properties, and CSS Parts applied in your parent context will always apply to your overlaid content.
@@ -790,9 +772,9 @@ When nesting multiple overlays, it is important to ensure that the nested overla
790772

791773
The overlay manages focus based on its type:
792774

793-
- For `modal` and `page` types, focus is always trapped within the overlay
794-
- For `auto` and `manual` types, focus behavior is controlled by the `receives-focus` attribute
795-
- For `hint` type, focus remains on the trigger element
775+
- For `modal` and `page` types, focus is always trapped within the overlay
776+
- For `auto` and `manual` types, focus behavior is controlled by the `receives-focus` attribute
777+
- For `hint` type, focus remains on the trigger element
796778

797779
Example of proper focus management:
798780

@@ -858,10 +840,10 @@ Example of proper focus management:
858840

859841
#### Screen reader considerations
860842

861-
- Use `aria-haspopup` on trigger elements to indicate the type of overlay
862-
- Provide descriptive labels using `aria-label` or `aria-labelledby`
863-
- Use proper heading structure within overlays
864-
- Ensure error messages are announced using `aria-live`
843+
- Use `aria-haspopup` on trigger elements to indicate the type of overlay
844+
- Provide descriptive labels using `aria-label` or `aria-labelledby`
845+
- Use proper heading structure within overlays
846+
- Ensure error messages are announced using `aria-live`
865847

866848
Example of a tooltip with proper screen reader support:
867849

packages/overlay/src/Overlay.ts

Lines changed: 1 addition & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,6 @@ if (!browserSupportsPopover) {
7979
* @attr {string} receives-focus - How focus should be handled ('true'|'false'|'auto')
8080
* @attr {boolean} delayed - Whether the overlay should wait for a warm-up period before opening
8181
* @attr {boolean} open - Whether the overlay is currently open
82-
* @attr {boolean} allow-outside-click - @deprecated Whether clicks outside the overlay should close it (not recommended for accessibility)
8382
*/
8483
export class Overlay extends ComputedOverlayBase {
8584
static override styles = [styles];
@@ -290,18 +289,6 @@ export class Overlay extends ComputedOverlayBase {
290289
@property({ attribute: 'receives-focus' })
291290
override receivesFocus: 'true' | 'false' | 'auto' = 'auto';
292291

293-
/**
294-
* @deprecated This property will be removed in a future version.
295-
* We do not recommend using this property for accessibility reasons.
296-
* It allows clicks outside the overlay to close it, which can cause
297-
* unexpected behavior and accessibility issues.
298-
*
299-
* @type {boolean}
300-
* @default false
301-
*/
302-
@property({ type: Boolean, attribute: 'allow-outside-click' })
303-
allowOutsideClick = false;
304-
305292
/**
306293
* A reference to the slot element within the overlay.
307294
*
@@ -508,6 +495,7 @@ export class Overlay extends ComputedOverlayBase {
508495
*
509496
* This method handles the necessary steps to open the popover, including managing delays,
510497
* ensuring the popover is in the DOM, making transitions, and applying focus.
498+
*
511499
* @protected
512500
* @override
513501
* @returns {Promise<void>} A promise that resolves when the popover has been fully opened.
@@ -564,7 +552,6 @@ export class Overlay extends ComputedOverlayBase {
564552
},
565553
// disable escape key capture to close the overlay, the focus-trap library captures it otherwise
566554
escapeDeactivates: false,
567-
allowOutsideClick: this.allowOutsideClick,
568555
});
569556

570557
if (this.type === 'modal' || this.type === 'page') {
@@ -985,23 +972,6 @@ export class Overlay extends ComputedOverlayBase {
985972
);
986973
}
987974

988-
// Warn about deprecated allowOutsideClick property
989-
if (changes.has('allowOutsideClick') && this.allowOutsideClick) {
990-
if (window.__swc?.DEBUG) {
991-
window.__swc.warn(
992-
this,
993-
`The "allow-outside-click" attribute on <${this.localName}> has been deprecated and will be removed in a future release. We do not recommend using this attribute for accessibility reasons. It allows clicks outside the overlay to close it, which can cause unexpected behavior and accessibility issues.`,
994-
'https://opensource.adobe.com/spectrum-web-components/components/overlay/',
995-
{ level: 'deprecation' }
996-
);
997-
} else {
998-
// Fallback for testing environments or when SWC is not available
999-
console.warn(
1000-
`[${this.localName}] The "allow-outside-click" attribute has been deprecated and will be removed in a future release. We do not recommend using this attribute for accessibility reasons. It allows clicks outside the overlay to close it, which can cause unexpected behavior and accessibility issues.`
1001-
);
1002-
}
1003-
}
1004-
1005975
// Manage the open state if the 'open' property has changed.
1006976
if (changes.has('open') && (this.hasUpdated || this.open)) {
1007977
this.manageOpen(changes.get('open'));

packages/overlay/test/overlay.test.ts

Lines changed: 0 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1138,38 +1138,3 @@ describe('Overlay should correctly trap focus', () => {
11381138
expect(document.activeElement).to.equal(input);
11391139
});
11401140
});
1141-
1142-
describe('Overlay - Deprecated Properties', () => {
1143-
it('should support allowOutsideClick property with deprecation warning', async () => {
1144-
const consoleSpy = spy(console, 'warn');
1145-
1146-
const el = await fixture<HTMLDivElement>(html`
1147-
<div>
1148-
<sp-button id="trigger">Open Overlay</sp-button>
1149-
<sp-overlay
1150-
trigger="trigger@click"
1151-
type="auto"
1152-
?allow-outside-click=${true}
1153-
>
1154-
<sp-popover dialog>
1155-
<p>Overlay content</p>
1156-
</sp-popover>
1157-
</sp-overlay>
1158-
</div>
1159-
`);
1160-
1161-
const overlay = el.querySelector('sp-overlay') as Overlay;
1162-
await elementUpdated(overlay);
1163-
1164-
// Verify the property is set correctly
1165-
expect(overlay.allowOutsideClick).to.be.true;
1166-
expect(overlay.hasAttribute('allow-outside-click')).to.be.true;
1167-
1168-
// Verify the deprecation warning is shown (either via SWC or console.warn fallback)
1169-
expect(consoleSpy.calledOnce).to.be.true;
1170-
expect(consoleSpy.firstCall.args[0]).to.include('allow-outside-click');
1171-
expect(consoleSpy.firstCall.args[0]).to.include('deprecated');
1172-
1173-
consoleSpy.restore();
1174-
});
1175-
});

0 commit comments

Comments
 (0)