+
Skip to content
This repository was archived by the owner on Oct 14, 2025. It is now read-only.

Conversation

clintcs
Copy link
Collaborator

@clintcs clintcs commented May 14, 2024

🚀 Description

N/A

📋 Checklist

  • I have read and followed the Contributing Guidelines.
  • I have added tests to cover new or updated functionality.
  • I have created or updated stories in Storybook to document the new functionality.
  • I have included a changeset with this Pull Request if it adds/updates/removes functionality for consumers.
  • I have scheduled a Design Review for these changes, if one is required.
  • I have followed the ARIA Authoring Practices Guide and/or met with the Accessibility Team to ensure this functionality is accessible.
  • I have added the component to exports in packages/components/package.json (if applicable).

🔬 How to Test

Open Storybook and play around with the placement options.

📸 Images/Videos of Functionality

N/A

Copy link

changeset-bot bot commented May 14, 2024

🦋 Changeset detected

Latest commit: 2731196

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@crowdstrike/glide-core-components Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@clintcs clintcs force-pushed the expose-menu-placement-attribute branch from c356292 to f968420 Compare May 14, 2024 14:22
set open(isOpen) {
this.#isOpen = isOpen;

if (isOpen) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sure Menu responds to assignments to placement after render. It also ensures Floating UI isn't doing stuff in the background when Menu is closed.

Copy link
Contributor

@clintcs clintcs force-pushed the expose-menu-placement-attribute branch 2 times, most recently from ca7069e to 39bfd54 Compare May 14, 2024 14:31
return html`<div style="height: 8rem;">
<cs-menu size=${arguments_.size || nothing} ?open=${arguments_.open}>
return html`<div
style="height: 13rem; display: flex; align-items: center; justify-content: center;"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make room for the various placement options.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only case I know of where we were setting the default this way.

@clintcs clintcs marked this pull request as ready for review May 14, 2024 14:37
@clintcs clintcs force-pushed the expose-menu-placement-attribute branch from 39bfd54 to 2731196 Compare May 14, 2024 15:29
@clintcs clintcs merged commit da5e41e into main May 14, 2024
@clintcs clintcs deleted the expose-menu-placement-attribute branch May 14, 2024 15:34
@github-actions github-actions bot mentioned this pull request May 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载