Skip to content

Commit 0946196

Browse files
authored
Merge branch 'main' into nikkimk/swc-1326-a11y-guides
2 parents c55127d + f07344f commit 0946196

File tree

14 files changed

+472
-81
lines changed

14 files changed

+472
-81
lines changed
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
---
2+
'@spectrum-web-components/overlay': patch
3+
'@spectrum-web-components/picker': patch
4+
---
5+
6+
**Fixed** issue where picker menus inside overlays could not scroll to the bottom after selecting an item and reopening. The problem was caused by the overlay's placement calculation happening before the menu fully rendered, resulting in incorrect height measurements.
7+
8+
This fix ensures picker menus maintain proper scrollable height when reopened, regardless of the selected item's position.

.changeset/real-corners-cross.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@spectrum-web-components/tooltip': patch
3+
---
4+
5+
**Fixed**: Fixed an issue with text overflow in `sp-tooltip`: long, unbroken words were not wrapping and overflowed the container.

1st-gen/packages/action-menu/src/ActionMenu.ts

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -148,19 +148,21 @@ export class ActionMenu extends ObserveSlotPresence(
148148
}
149149

150150
protected override warnNoLabel(): void {
151-
window.__swc.warn(
152-
this,
153-
`<${this.localName}> needs one of the following to be accessible:`,
154-
'https://opensource.adobe.com/spectrum-web-components/components/action-menu/#accessibility',
155-
{
156-
type: 'accessibility',
157-
issues: [
158-
`an <sp-field-label> element with a \`for\` attribute referencing the \`id\` of the \`<${this.localName}>\`, or`,
159-
'value supplied to the "label" attribute, which will be displayed visually as placeholder text',
160-
'text content supplied in a <span> with slot="label", or, text content supplied in a <span> with slot="label-only"',
161-
'which will also be displayed visually as placeholder text.',
162-
],
163-
}
164-
);
151+
if (window.__swc?.DEBUG) {
152+
window.__swc.warn(
153+
this,
154+
`<${this.localName}> needs one of the following to be accessible:`,
155+
'https://opensource.adobe.com/spectrum-web-components/components/action-menu/#accessibility',
156+
{
157+
type: 'accessibility',
158+
issues: [
159+
`an <sp-field-label> element with a \`for\` attribute referencing the \`id\` of the \`<${this.localName}>\`, or`,
160+
'value supplied to the "label" attribute, which will be displayed visually as placeholder text',
161+
'text content supplied in a <span> with slot="label", or, text content supplied in a <span> with slot="label-only"',
162+
'which will also be displayed visually as placeholder text.',
163+
],
164+
}
165+
);
166+
}
165167
}
166168
}

1st-gen/packages/menu/src/Menu.ts

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -103,30 +103,30 @@ export class Menu extends SizedMixin(SpectrumElement, { noDefaultSize: true }) {
103103

104104
/**
105105
* Minimum vertical movement (in pixels) required to trigger scrolling detection.
106-
*
106+
*
107107
* This threshold is consistent with other components in the design system:
108108
* - Card component uses 10px for click vs. drag detection
109109
* - Menu component uses 10px for scroll vs. selection detection
110-
*
110+
*
111111
* The 10px threshold is carefully chosen to:
112112
* - Allow for natural finger tremor and accidental touches
113113
* - Distinguish between intentional scroll gestures and taps
114114
* - Provide consistent behavior across the platform
115-
*
115+
*
116116
* @see {@link packages/card/src/Card.ts} for similar threshold usage
117117
*/
118118
private scrollThreshold = 10; // pixels
119119

120120
/**
121121
* Maximum time (in milliseconds) for a movement to be considered scrolling.
122-
*
122+
*
123123
* This threshold is consistent with other timing values in the design system:
124124
* - Longpress duration: 300ms (ActionButton, LongpressController)
125125
* - Scroll detection: 300ms (Menu component)
126-
*
126+
*
127127
* Quick movements within this timeframe are likely intentional scrolls,
128128
* while slower movements are more likely taps or selections.
129-
*
129+
*
130130
* @see {@link packages/action-button/src/ActionButton.ts} for longpress duration
131131
* @see {@link packages/overlay/src/LongpressController.ts} for longpress duration
132132
*/
@@ -576,7 +576,7 @@ export class Menu extends SizedMixin(SpectrumElement, { noDefaultSize: true }) {
576576
* Resets the scrolling state after a short delay (100ms) to allow for
577577
* any final touch events to be processed. This delay prevents immediate
578578
* state changes that could interfere with the selection logic.
579-
*
579+
*
580580
* The 100ms delay is consistent with the design system's approach to
581581
* touch event handling and ensures that any final touch events or
582582
* gesture recognition can complete before the scrolling state is reset.
@@ -720,7 +720,7 @@ export class Menu extends SizedMixin(SpectrumElement, { noDefaultSize: true }) {
720720
): MenuItem {
721721
const diff = before ? -1 : 1;
722722
const elements = this.rovingTabindexController?.elements || [];
723-
const index = !!menuItem ? elements.indexOf(menuItem) : -1;
723+
const index = menuItem ? elements.indexOf(menuItem) : -1;
724724
let newIndex = Math.min(Math.max(0, index + diff), elements.length - 1);
725725
while (
726726
!this.isFocusableElement(elements[newIndex]) &&
@@ -729,7 +729,7 @@ export class Menu extends SizedMixin(SpectrumElement, { noDefaultSize: true }) {
729729
) {
730730
newIndex += diff;
731731
}
732-
return !!this.isFocusableElement(elements[newIndex])
732+
return this.isFocusableElement(elements[newIndex])
733733
? (elements[newIndex] as MenuItem)
734734
: menuItem || elements[0];
735735
}
@@ -1010,7 +1010,7 @@ export class Menu extends SizedMixin(SpectrumElement, { noDefaultSize: true }) {
10101010
}
10111011
});
10121012
}
1013-
if (!!this._updateFocus) {
1013+
if (this._updateFocus) {
10141014
this.rovingTabindexController?.focusOnItem(this._updateFocus);
10151015
this._updateFocus = undefined;
10161016
}

1st-gen/packages/overlay/src/PlacementController.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import {
2424
shift,
2525
size,
2626
} from '@floating-ui/dom';
27+
import { isWebKit } from '@spectrum-web-components/shared';
2728
import type { VirtualTrigger } from './VirtualTrigger.js';
2829
import type { OpenableElement } from './overlay-types.js';
2930
import type { Overlay } from './Overlay.js';
@@ -278,6 +279,28 @@ export class PlacementController implements ReactiveController {
278279
// Wait for document fonts to be ready before computing placement.
279280
await (document.fonts ? document.fonts.ready : Promise.resolve());
280281

282+
// Safari/iOS-specific fix: Add small delay for picker menus to allow scrollIntoView to complete
283+
// Check if this is a submenu overlay (slot="submenu")
284+
// Submenus need immediate positioning for hover responsiveness
285+
const isSubmenu = Array.from(this.host.elements).some(
286+
(el) => el.getAttribute?.('slot') === 'submenu'
287+
);
288+
289+
// Safari-specific timing fix covered by cross-browser integration tests
290+
/* c8 ignore start */
291+
if (isWebKit() && !isSubmenu) {
292+
const hasMenu = Array.from(this.host.elements).some(
293+
(el) =>
294+
el.tagName === 'SP-MENU' || el.querySelector?.('sp-menu')
295+
);
296+
297+
if (hasMenu) {
298+
// Wait 1 frame for Safari layout to settle after scrollIntoView
299+
await new Promise((resolve) => requestAnimationFrame(resolve));
300+
}
301+
}
302+
/* c8 ignore stop */
303+
281304
// Determine the flip middleware based on the type of trigger element.
282305
const flipMiddleware = !(options.trigger instanceof HTMLElement)
283306
? flip({

1st-gen/packages/overlay/src/slottable-request-directive.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ export class SlottableRequestDirective extends AsyncDirective {
7474
{ signal }
7575
);
7676

77-
if (window.__swc.DEBUG) {
77+
if (window.__swc?.DEBUG) {
7878
window.__swc.warn(
7979
undefined,
8080
`⚠️ WARNING ⚠️ : The Overlay Trigger Directive is experimental and there is no guarantees behind its usage in an application!! Its API and presence within the library could be changed at anytime. See "sp-overlay" or "Overlay.open()" for a stable API for overlaying content on your application.`,

1st-gen/packages/overlay/src/slottable-request-event.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ export class SlottableRequestEvent extends Event {
2323
this.name = name;
2424
this.data = data;
2525
this.slotName = key !== undefined ? `${name}.${key}` : name;
26-
if (window.__swc.DEBUG) {
26+
if (window.__swc?.DEBUG) {
2727
window.__swc.warn(
2828
undefined,
2929
`⚠️ WARNING ⚠️ : \`slottable-request\` events are experimental and there is no guarantees behind usage of them in an application!! Their shape and presence within the library could be changed at anytime.

1st-gen/packages/overlay/test/overlay.test.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ import {
3535
import { render, TemplateResult } from '@spectrum-web-components/base';
3636
import { Button } from '@spectrum-web-components/button';
3737
import { Menu } from '@spectrum-web-components/menu';
38+
import '@spectrum-web-components/menu/sp-menu.js';
39+
import '@spectrum-web-components/menu/sp-menu-item.js';
3840
import { Theme } from '@spectrum-web-components/theme';
3941
import '@spectrum-web-components/theme/sp-theme.js';
4042
import '@spectrum-web-components/theme/src/themes.js';
@@ -54,7 +56,7 @@ import {
5456
clickAndHoverTarget,
5557
definedOverlayElement,
5658
virtualElement,
57-
} from '../stories/overlay.stories';
59+
} from '../stories/overlay.stories.js';
5860
// import { isWebKit } from '@spectrum-web-components/shared';
5961

6062
async function styledFixture<T extends Element>(
@@ -476,6 +478,7 @@ describe('Overlays', () => {
476478

477479
const initial = el.getBoundingClientRect();
478480
trigger.updateBoundingClientRect(500, 500);
481+
// Wait for placement computation to complete
479482
await nextFrame();
480483
await nextFrame();
481484
const final = el.getBoundingClientRect();

1st-gen/packages/picker/src/Picker.ts

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -556,19 +556,21 @@ export class PickerBase extends SizedMixin(SpectrumElement, {
556556
}
557557

558558
protected warnNoLabel(): void {
559-
window.__swc.warn(
560-
this,
561-
`<${this.localName}> needs one of the following to be accessible:`,
562-
'https://opensource.adobe.com/spectrum-web-components/components/picker/#accessibility',
563-
{
564-
type: 'accessibility',
565-
issues: [
566-
`an <sp-field-label> element with a \`for\` attribute referencing the \`id\` of the \`<${this.localName}>\`, or`,
567-
'value supplied to the "label" attribute, which will be displayed visually as placeholder text, or',
568-
'text content supplied in a <span> with slot="label", which will also be displayed visually as placeholder text.',
569-
],
570-
}
571-
);
559+
if (window.__swc?.DEBUG) {
560+
window.__swc.warn(
561+
this,
562+
`<${this.localName}> needs one of the following to be accessible:`,
563+
'https://opensource.adobe.com/spectrum-web-components/components/picker/#accessibility',
564+
{
565+
type: 'accessibility',
566+
issues: [
567+
`an <sp-field-label> element with a \`for\` attribute referencing the \`id\` of the \`<${this.localName}>\`, or`,
568+
'value supplied to the "label" attribute, which will be displayed visually as placeholder text, or',
569+
'text content supplied in a <span> with slot="label", which will also be displayed visually as placeholder text.',
570+
],
571+
}
572+
);
573+
}
572574
}
573575

574576
protected renderOverlay(menu: TemplateResult): TemplateResult {
@@ -1000,7 +1002,7 @@ export class Picker extends PickerBase {
10001002
);
10011003
if (!this.value || nextItem !== this.selectedItem) {
10021004
// updates picker text but does not fire change event until action is completed
1003-
if (!!nextItem) this.setValueFromItem(nextItem as MenuItem);
1005+
if (nextItem) this.setValueFromItem(nextItem as MenuItem);
10041006
}
10051007
};
10061008
}

0 commit comments

Comments
 (0)