Skip to content

Commit

Permalink
fix(overlay): clean position data on close
Browse files Browse the repository at this point in the history
  • Loading branch information
Westbrook committed Jan 24, 2024
1 parent 669f8e3 commit edac590
Show file tree
Hide file tree
Showing 7 changed files with 119 additions and 26 deletions.
43 changes: 39 additions & 4 deletions packages/action-menu/stories/action-menu.stories.ts
Expand Up @@ -10,6 +10,7 @@ OF ANY KIND, either express or implied. See the License for the specific languag
governing permissions and limitations under the License.
*/
import { html, TemplateResult } from '@spectrum-web-components/base';
import { ifDefined } from '@spectrum-web-components/base/src/directives.js';

import '@spectrum-web-components/action-menu/sp-action-menu.js';
import '@spectrum-web-components/menu/sp-menu.js';
Expand Down Expand Up @@ -137,8 +138,25 @@ export default {
options: ['white', 'black', 'none'],
},
},
align: {
name: 'align',
type: { name: 'string', required: false },
description: 'Alignment of the Action Menu',
table: {
defaultValue: { summary: 'start' },
},
control: {
type: 'select',
labels: {
start: 'start',
end: 'end',
},
},
options: ['start', 'end'],
},
},
args: {
align: 'start',
visibleLabel: 'More Actions',
disabled: false,
open: false,
Expand All @@ -150,6 +168,7 @@ export default {
};

interface StoryArgs {
align?: 'start' | 'end';
visibleLabel?: string;
disabled?: boolean;
open?: boolean;
Expand Down Expand Up @@ -185,6 +204,7 @@ quiet.args = {
};

export const labelOnly = ({
align = 'start',
changeHandler = (() => undefined) as (event: Event) => void,
disabled = false,
open = false,
Expand All @@ -202,6 +222,7 @@ export const labelOnly = ({
}}
.selects=${selects ? selects : undefined}
value=${selected ? 'Select Inverse' : ''}
style=${ifDefined(align === 'end' ? 'float: inline-end;' : undefined)}
>
<span slot="label-only">Label Only</span>
<sp-menu-item>Deselect</sp-menu-item>
Expand Down Expand Up @@ -248,9 +269,14 @@ customIcon.args = {
visibleLabel: '',
};

export const submenu = (): TemplateResult => {
export const submenu = ({ align = 'start' } = {}): TemplateResult => {
return html`
<sp-action-menu label="More Actions">
<sp-action-menu
label="More Actions"
style=${ifDefined(
align === 'end' ? 'float: inline-end;' : undefined
)}
>
<sp-menu-item>One</sp-menu-item>
<sp-menu-item>Two</sp-menu-item>
<sp-menu-item>
Expand All @@ -265,7 +291,7 @@ export const submenu = (): TemplateResult => {
`;
};

export const controlled = (): TemplateResult => {
export const controlled = ({ align = 'start' } = {}): TemplateResult => {
const state = {
snap: true,
grid: false,
Expand All @@ -292,7 +318,13 @@ export const controlled = (): TemplateResult => {
)!.textContent = `application state: ${JSON.stringify(state)}`;
}
return html`
<sp-action-menu label="View" @change=${onChange}>
<sp-action-menu
label="View"
@change=${onChange}
style=${ifDefined(
align === 'end' ? 'float: inline-end;' : undefined
)}
>
<sp-menu-item value="action" @click=${() => alert('action')}>
Non-selectable action
</sp-menu-item>
Expand Down Expand Up @@ -332,14 +364,17 @@ export const controlled = (): TemplateResult => {
};

export const groups = ({
align = 'start',
onChange,
}: {
align: 'start' | 'end';
onChange(value: string): void;
}): TemplateResult => html`
<sp-action-menu
@change=${({ target: { value } }: Event & { target: ActionMenu }) =>
onChange(value)}
open
style=${ifDefined(align === 'end' ? 'float: inline-end;' : undefined)}
>
<sp-menu-group id="cms">
<span slot="header">cms</span>
Expand Down
4 changes: 4 additions & 0 deletions packages/action-menu/stories/index.ts
Expand Up @@ -22,6 +22,7 @@ import { Placement } from '@spectrum-web-components/overlay/src/overlay-types.js
import type { ActionMenu } from '@spectrum-web-components/action-menu';

export const ActionMenuMarkup = ({
align = 'start',
ariaLabel = 'More Actions',
onChange = (() => undefined) as (value: string) => void,
changeHandler = (() => undefined) as (value: string) => void,
Expand Down Expand Up @@ -55,6 +56,9 @@ export const ActionMenuMarkup = ({
}}
.selects=${selects ? selects : undefined}
value=${selected ? 'Select Inverse' : ''}
style=${ifDefined(
align === 'end' ? 'float: inline-end;' : undefined
)}
>
${customIcon ? customIcon : nothing}
${visibleLabel
Expand Down
46 changes: 45 additions & 1 deletion packages/action-menu/test/index.ts
Expand Up @@ -29,12 +29,16 @@ import {
ignoreResizeObserverLoopError,
} from '../../../test/testing-helpers.js';
import '@spectrum-web-components/dialog/sp-dialog-base.js';
import { tooltipDescriptionAndPlacement } from '../stories/action-menu.stories';
import {
iconOnly,
tooltipDescriptionAndPlacement,
} from '../stories/action-menu.stories.js';
import { findDescribedNode } from '../../../test/testing-helpers-a11y.js';
import type { Tooltip } from '@spectrum-web-components/tooltip';
import { sendMouse } from '../../../test/plugins/browser.js';
import type { TestablePicker } from '../../picker/test/index.js';
import type { Overlay } from '@spectrum-web-components/overlay';
import { sendKeys } from '@web/test-runner-commands';

ignoreResizeObserverLoopError(before, after);

Expand Down Expand Up @@ -307,6 +311,46 @@ export const testActionMenu = (mode: 'sync' | 'async'): void => {
expect(button).to.have.attribute('aria-expanded', 'true');
expect(button).to.have.attribute('aria-controls', 'menu');
});
it('opens repeatedly with Menu in the correct location', async function () {
const el = await fixture<ActionMenu>(
iconOnly({
...iconOnly.args,
align: 'end',
})
);

await elementUpdated(el);

el.focus();
await elementUpdated(el);
let opened = oneEvent(el, 'sp-opened');
await sendKeys({ press: 'ArrowRight' });
await sendKeys({ press: 'ArrowLeft' });
await sendKeys({ press: 'Space' });
await opened;

const firstRect = (
el as unknown as { overlayElement: Overlay }
).overlayElement.dialogEl.getBoundingClientRect();

let closed = oneEvent(el, 'sp-closed');
await sendKeys({ press: 'Space' });
await closed;

opened = oneEvent(el, 'sp-opened');
await sendKeys({ press: 'Space' });
await opened;

const secondRect = (
el as unknown as { overlayElement: Overlay }
).overlayElement.dialogEl.getBoundingClientRect();

closed = oneEvent(el, 'sp-closed');
await sendKeys({ press: 'Space' });
await closed;

expect(firstRect).to.deep.equal(secondRect);
});
it('opens and selects in a single pointer button interaction', async () => {
const el = await actionMenuFixture();
const thirdItem = el.querySelector(
Expand Down
4 changes: 2 additions & 2 deletions packages/overlay/package.json
Expand Up @@ -126,8 +126,8 @@
"lit-html"
],
"dependencies": {
"@floating-ui/dom": "^1.5.3",
"@floating-ui/utils": "^0.1.6",
"@floating-ui/dom": "^1.5.4",
"@floating-ui/utils": "^0.2.1",
"@spectrum-web-components/action-button": "^0.40.3",
"@spectrum-web-components/base": "^0.40.3",
"@spectrum-web-components/reactive-controllers": "^0.40.3",
Expand Down
3 changes: 3 additions & 0 deletions packages/overlay/src/Overlay.ts
Expand Up @@ -957,6 +957,9 @@ export class Overlay extends OverlayFeatures {
this.placementController.resetOverlayPosition();
}
}
if (changes.has('state') && this.state === 'closed') {
this.placementController.clearOverlayPosition();
}
}

protected renderContent(): TemplateResult {
Expand Down
15 changes: 11 additions & 4 deletions packages/overlay/src/PlacementController.ts
Expand Up @@ -267,13 +267,20 @@ export class PlacementController implements ReactiveController {
}
}

public resetOverlayPosition = (): void => {
if (!this.target || !this.options) return;

public clearOverlayPosition(): void {
if (!this.target) {
return;
}
this.target.style.removeProperty('max-height');
this.target.style.removeProperty('height');
this.target.style.removeProperty('max-width');
this.initialHeight = undefined;
this.isConstrained = false;
}

public resetOverlayPosition = (): void => {
if (!this.target || !this.options) return;
this.clearOverlayPosition();

// force paint
this.host.offsetHeight;
this.computePlacement();
Expand Down
30 changes: 15 additions & 15 deletions yarn.lock
Expand Up @@ -3724,12 +3724,12 @@
dependencies:
"@floating-ui/utils" "^0.1.1"

"@floating-ui/core@^1.4.2":
version "1.5.0"
resolved "https://registry.yarnpkg.com/@floating-ui/core/-/core-1.5.0.tgz#5c05c60d5ae2d05101c3021c1a2a350ddc027f8c"
integrity sha512-kK1h4m36DQ0UHGj5Ah4db7R0rHemTqqO0QLvUqi1/mUUp3LuAWbWxdxSIf/XsnH9VS6rRVPLJCncjRzUvyCLXg==
"@floating-ui/core@^1.5.3":
version "1.5.3"
resolved "https://registry.yarnpkg.com/@floating-ui/core/-/core-1.5.3.tgz#b6aa0827708d70971c8679a16cf680a515b8a52a"
integrity sha512-O0WKDOo0yhJuugCx6trZQj5jVJ9yR0ystG2JaNAemYUWce+pmM6WUEFIibnWyEJKdrDxhm75NoSRME35FNaM/Q==
dependencies:
"@floating-ui/utils" "^0.1.3"
"@floating-ui/utils" "^0.2.0"

"@floating-ui/dom@^1.5.1":
version "1.5.1"
Expand All @@ -3739,13 +3739,13 @@
"@floating-ui/core" "^1.4.1"
"@floating-ui/utils" "^0.1.1"

"@floating-ui/dom@^1.5.3":
version "1.5.3"
resolved "https://registry.yarnpkg.com/@floating-ui/dom/-/dom-1.5.3.tgz#54e50efcb432c06c23cd33de2b575102005436fa"
integrity sha512-ClAbQnEqJAKCJOEbbLo5IUlZHkNszqhuxS4fHAVxRPXPya6Ysf2G8KypnYcOTpx6I8xcgF9bbHb6g/2KpbV8qA==
"@floating-ui/dom@^1.5.4":
version "1.5.4"
resolved "https://registry.yarnpkg.com/@floating-ui/dom/-/dom-1.5.4.tgz#28df1e1cb373884224a463235c218dcbd81a16bb"
integrity sha512-jByEsHIY+eEdCjnTVu+E3ephzTOzkQ8hgUfGwos+bg7NlH33Zc5uO+QHz1mrQUOgIKKDD1RtS201P9NvAfq3XQ==
dependencies:
"@floating-ui/core" "^1.4.2"
"@floating-ui/utils" "^0.1.3"
"@floating-ui/core" "^1.5.3"
"@floating-ui/utils" "^0.2.0"

"@floating-ui/react-dom@^2.0.0":
version "2.0.2"
Expand All @@ -3759,10 +3759,10 @@
resolved "https://registry.yarnpkg.com/@floating-ui/utils/-/utils-0.1.1.tgz#1a5b1959a528e374e8037c4396c3e825d6cf4a83"
integrity sha512-m0G6wlnhm/AX0H12IOWtK8gASEMffnX08RtKkCgTdHb9JpHKGloI7icFfLg9ZmQeavcvR0PKmzxClyuFPSjKWw==

"@floating-ui/utils@^0.1.3", "@floating-ui/utils@^0.1.6":
version "0.1.6"
resolved "https://registry.yarnpkg.com/@floating-ui/utils/-/utils-0.1.6.tgz#22958c042e10b67463997bd6ea7115fe28cbcaf9"
integrity sha512-OfX7E2oUDYxtBvsuS4e/jSn4Q9Qb6DzgeYtsAdkPZ47znpoNsMgZw0+tVijiv3uGNR6dgNlty6r9rzIzHjtd/A==
"@floating-ui/utils@^0.2.0", "@floating-ui/utils@^0.2.1":
version "0.2.1"
resolved "https://registry.yarnpkg.com/@floating-ui/utils/-/utils-0.2.1.tgz#16308cea045f0fc777b6ff20a9f25474dd8293d2"
integrity sha512-9TANp6GPoMtYzQdt54kfAyMmz1+osLlXdg2ENroU7zzrtflTLrrC/lgrIfaSe+Wu0b89GKccT7vxXA0MoAIO+Q==

"@formatjs/ecma402-abstract@1.14.3":
version "1.14.3"
Expand Down

0 comments on commit edac590

Please sign in to comment.