-
Notifications
You must be signed in to change notification settings - Fork 8
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
Replace Text Fields with Dropdowns for Enum Parameters in API Sandbox UI #306
base: main
Are you sure you want to change the base?
Conversation
…into ari/zudoku-1
- Add enum property to pathParams and headers in PlaygroundForm - Update QueryParams component to handle enum values in Select component
…ri/enum-string-dropdown
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
@arian0zen is attempting to deploy a commit to the Zuplo WWW Team on Vercel. A member of the Team first needs to authorize it. |
Awesome. Thanks for making quick work of this issue. |
Thanks for your contribution @arian0zen! Looks great so far 🙏 Can you provide the changes you made to the OpenAPI schema so we can test it? (and sorry for the merging all the time, the CI doesn't work properly with forks yet) |
🫱🏽🫲🏾 I am very much open to any suggestions, let me know if any changes are required! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work! Thanks for the deep dive into the code base and the changes you’re proposing!
Some things:
- With the select fields now, when we select/change the values the table columns jump and change the layout. Can we avoid this?
- The multi select is a tricky component that is hard to get exactly right, e.g., some things I noticed:
- When selecting a value, the dialog closes. I think it should stay open
- I can’t unselect the last item if there’s only one item selected
- A selected item should not have a background color because you can’t see hovered/active item. The checkmark should suffice
- Maybe we can get some inspiration from here: Multi select ? shadcn-ui/ui#66 (comment). E.g. this implementation: https://shadcnui-expansions.typeart.cc/docs/multiple-selector.
I don’t want to block this but I think if we add such a versatile component it should be solid :)
Because we want to expose the components to be used by Zudoku users (in Markdown, custom pages, etc.)
<SelectTrigger | ||
className="w-full flex" | ||
placeholder="Choose an option" | ||
value={field.value} | ||
> | ||
<SelectValue /> | ||
</SelectTrigger> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's just use placeholder
that SelectValue
provides. Then we don't need to add this prop to SelectTrigger
So:
<SelectTrigger | |
className="w-full flex" | |
placeholder="Choose an option" | |
value={field.value} | |
> | |
<SelectValue /> | |
</SelectTrigger> | |
<SelectTrigger | |
className="w-full flex" | |
value={field.value} | |
> | |
<SelectValue placeholder="Choose an option" /> | |
</SelectTrigger> |
)) | ||
) : ( | ||
<Fragment key={p.name}> | ||
{p.name}={encodeURIComponent(p.value)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should leave the replacement for %20
to +
in.
@dan-lee these are very solid suggestions. Working on it🙌 |
This PR introduces dropdowns and multi-selects in the Zudoku sandbox UI for parameters with constrained values, using the enum metadata from the OpenAPI spec. Instead of displaying text fields for these parameters, the updated UI now automatically populates dropdowns or multi-selects, streamlining the experience for users exploring the API.
This enhancement addresses issue ZUP-3767.
🎥 Watch the Demo