feat: add components to design system batch #1#1703
feat: add components to design system batch #1#1703isadorable-png wants to merge 9 commits intodevfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: decff100ad
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| <div | ||
| role="option" | ||
| aria-selected={isSelected} | ||
| aria-disabled={disabled} | ||
| onClick={disabled ? undefined : onClick} |
There was a problem hiding this comment.
Make combobox options keyboard-focusable
These options are rendered as plain div elements with only mouse handlers, so once the listbox opens, keyboard users cannot focus an option or select it with Enter/Space. In practice this makes the combobox unusable without a mouse and contradicts the role="listbox"/role="option" semantics exposed by this component.
Useful? React with 👍 / 👎.
apps/dashboard/app/globals.css
Outdated
| transform: translateY(20px); | ||
| } | ||
| to { | ||
| opacity: 1; | ||
| transform: translateY(0); | ||
| } |
There was a problem hiding this comment.
Preserve centering transform in modal keyframes
The modal content is centered with -translate-x-1/2 -translate-y-1/2, but these keyframes set transform to only translateY(...), which overrides the centering transform during open/close animation. This causes the dialog to animate from an offset position (shifted right/down) instead of staying centered.
Useful? React with 👍 / 👎.
| <button | ||
| role="tab" | ||
| aria-selected={isActive} | ||
| onClick={onClick} |
There was a problem hiding this comment.
Set tab button type to avoid form submission
This <button> omits type="button", so inside any surrounding <form> it defaults to type="submit". Clicking a tab in that context will submit the form unexpectedly (and can trigger navigation/validation) instead of only changing tab state.
Useful? React with 👍 / 👎.
apps/dashboard/shared/components/design-system/combobox/combobox-item/ComboboxItem.figma.tsx
Outdated
Show resolved
Hide resolved
apps/dashboard/shared/components/design-system/combobox/combobox-item/ComboboxItem.tsx
Outdated
Show resolved
Hide resolved
| value: "uniswap", | ||
| label: "Uniswap", | ||
| icon: ( | ||
| <CrownIcon style={{ width: 14, height: 14 }} className="text-highlight" /> |
There was a problem hiding this comment.
same here do not use inline styles
| value, | ||
| placeholder = "Select…", | ||
| onValueChange, | ||
| disabled = false, |
There was a problem hiding this comment.
change this to isDisabled to follow dashboard patterns
| "text-secondary shrink-0 transition-transform duration-150", | ||
| open && "rotate-180", | ||
| )} | ||
| style={{ width: 14, height: 14 }} |
| }, | ||
| decorators: [ | ||
| (Story) => ( | ||
| <div className="w-[600px]"> |
There was a problem hiding this comment.
avoid explicit number like this, use tailwind classes
| actionsNumber = "1", | ||
| cancelLabel = "Cancel", | ||
| confirmLabel = "Confirm", |
There was a problem hiding this comment.
we dont need the actionsNumber prop, because if you pass cancelLabel you will have a cancel button, the same with confirm
| }, | ||
| decorators: [ | ||
| (Story) => ( | ||
| <div className="w-[270px]"> |
| const labelEl = label ? ( | ||
| <span className="text-secondary shrink-0 whitespace-nowrap text-sm leading-5"> | ||
| {label} | ||
| </span> | ||
| ) : null; | ||
|
|
||
| const trackEl = ( |
There was a problem hiding this comment.
what EL means? try to make this more legible
| )} | ||
| > | ||
| {label} | ||
| {items !== undefined && ( |
There was a problem hiding this comment.
| {items !== undefined && ( | |
| {!!items && items.length > 0 && ( |
…anticapture into feat/ds-new-components-batch-1
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 33541861ee
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| className, | ||
| )} | ||
| > | ||
| <Button variant="outline" size="md" onClick={onCancel}> |
There was a problem hiding this comment.
Set cancel action to non-submit button
This footer action is rendered through Button, which ultimately outputs a native <button> without an explicit type; in a surrounding <form>, that defaults to submit. As a result, clicking Cancel can trigger unintended form submission/navigation instead of only dismissing the modal, which is especially likely when modal content is used in form workflows.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 09858c7438
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| </PopoverPrimitive.Trigger> | ||
|
|
||
| {/* Dropdown content */} | ||
| <PopoverPrimitive.Content |
There was a problem hiding this comment.
Wrap combobox content in Popover portal
The dropdown is rendered as plain PopoverPrimitive.Content inside the local DOM tree, so when this component is placed inside containers with clipping (overflow-hidden, scroll regions, transformed parents), the option list can be partially or fully hidden even though the trigger is visible. That makes selection impossible in common dashboard layouts and is why other popovers here use a portal.
Useful? React with 👍 / 👎.
| <PillTab | ||
| key={tab.value} | ||
| label={tab.label} | ||
| isActive={activeTab === tab.value} | ||
| onClick={() => onTabChange?.(tab.value)} |
There was a problem hiding this comment.
Pass counter data through PillTabGroup
PillTab supports a counter payload, but PillTabGroup only forwards label and active state, so counter metadata is silently dropped whenever tabs are rendered through the group abstraction. In practice, grouped pill tabs cannot show voter/VP/percentage values even if callers have that data.
Useful? React with 👍 / 👎.
| @@ -0,0 +1,154 @@ | |||
| # New Component Checklist | |||
There was a problem hiding this comment.
missing name and description as defined by agent skills standard
No description provided.