Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c616fa0ec4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| onOpenChange={(isOpen) => { | ||
| if (!isOpen) { | ||
| onClose(); |
There was a problem hiding this comment.
Add interaction coverage for the migrated close path
The controlled close behavior now depends on Vaul invoking onOpenChange(false), but every existing BuildLogsDrawer.stories.tsx story lacks a play function, while the new generic stories only open an uncontrolled drawer and never exercise this callback. Add a build-log drawer story that closes it through the X button and another supported dismissal path, then verifies onClose, so regressions in this migration are covered.
AGENTS.md reference: site/AGENTS.md:L9-L10
Useful? React with 👍 / 👎.
|
@jakehwll we should probably take Codex's suggestion and author some stories with |
aslilac
left a comment
There was a problem hiding this comment.
I'm quite worried by this:
https://github.com/emilkowalski/vaul
This repo is unmaintained. I might come back to it at some point, but not in the near future. This was and always will be a hobby project and I simply don't have the time or will to work on it right now.
this is the only place in the ui where we have this specific kind of "drawer" concept, so I'm wondering...
- could we just get an agent to whip up something dead simple that covers exactly this one case well?
- could we use a different but close enough component we've already got? maybe reuse the same equivalent from the template version editor page?
- does base-ui provide something appropriate here that we could use?
|
Closing in favor of #28009 |
Adds a shared shadcn/Vaul
Drawerprimitive and migrates Create TemplateBuildLogsDraweroff MUI.Keeps the desktop panel at 800px and uses
min(800px, 100%)so it stays within the viewport on mobile. Includes Storybook coverage for the new component exports.