Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: S2 examples Popover and add CustomDialog #7459

Merged
merged 3 commits into from
Dec 4, 2024
Merged

Conversation

ktabors
Copy link
Member

@ktabors ktabors commented Dec 3, 2024

from testing

Changes:

  • Updated popover example to use Popover. Do we want it styled like S1 popovers? The implemented minimal changes,
    image
  • Added an example of CustomDialog
  • Changed ButtonGroup to have a maxWidth of the view width so the buttons will wrap on mobile.
  • Added viewport metadata to the parcel example app

Possible Bug: If we don't define size on CustomDialog it takes the full screen width.

✅ Pull Request Checklist:

  • Included link to corresponding React Spectrum GitHub Issue.
  • Added/updated unit tests and storybook for this change (for new code or code which already has tests).
  • Filled out test instructions.
  • Updated documentation (if it already exists for this component).
  • Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

For the Popover and CustomDialog dialog changes those can be tested on desktop.
For the parcel example app please test on mobile for the viewport and ButtonGroup changes. The viewport test is pickers open below the trigger.

When testing the webpack app, is there a ResizeObserver error on scroll down? I thought it was related to the ButtonGroup change, but I removed the change and it persisted. I'm hoping it's a local build issue. That's the only change in that file and I wouldn't expect changes to Lazy.jsx to impact this.

Speaking of local build issue, my parcel build is refusing to import Popover and CustomDialog. I'm assuming it's a local issue and no one else will see it.

🧢 Your Project:

RSP

@rspbot
Copy link

rspbot commented Dec 3, 2024

@ktabors ktabors added this pull request to the merge queue Dec 4, 2024
Merged via the queue into main with commit 5757c3a Dec 4, 2024
30 checks passed
@ktabors ktabors deleted the s2examples_popover branch December 4, 2024 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants