fix: bugs in dao creation flow#959
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughSyncs allocation toggle state and mobile form submission, adds an artwork reset confirmation modal that clears Formik and store state, standardizes feed image skeletons via a fullWidth prop, and updates button disabled handling and theme/color-token behavior. ChangesForm State & Navigation Refactoring
Artwork Reset Feature
Feed Image Skeleton Standardization
Design System Button & Theme Enhancements
Modal & Minter UI Updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/feed-ui/src/ZoraCoinCreatedItem.tsxESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. packages/feed-ui/src/ZoraDropCreatedItem.tsxESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. packages/ui/src/Artwork/ArtworkUpload.tsxESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/zord/src/elements/Button.tsx (1)
118-146:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
handleClickonly suppressesonClick, not native element actions.This gating works for the default
<button>(nativedisabledblocks activation), butButtonis polymorphic. Combined with the removal ofpointerEvents: 'none'inButton.css.ts, a disabledButton as="a" href=...would still navigate on click sincedisabledis inert on an anchor andhandleClickdoes not callpreventDefault. See the root-cause comment onButton.css.ts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/zord/src/elements/Button.tsx` around lines 118 - 146, The current useMemo for handleClick only returns undefined when disabled, which doesn't stop native element behavior for polymorphic elements (e.g., anchors); change handleClick so that when disabled it returns an actual event handler that calls event.preventDefault() and event.stopPropagation() (and does not call the original onClick), and when not disabled returns the provided onClick; update the handler signature to accept the MouseEvent used by the Button component so this safe-no-op prevents navigation for Button elements rendered as anchors while preserving normal behavior for real buttons.packages/feed-ui/src/ZoraCoinCreatedItem.tsx (1)
44-47:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInconsistent skeleton sizing during metadata loading.
Line 46 uses
<ImageSkeleton />withoutfullWidth, while Line 64 uses<ImageSkeleton fullWidth />for the FallbackImage placeholder. This creates inconsistent placeholder sizing between the metadata loading phase (lines 42-47) and the image loading phase (lines 60-67). Users will see the skeleton change width between loading states.🎨 Proposed fix for consistent sizing
) : ( <Box className={feedItemImage}> - <ImageSkeleton /> + <ImageSkeleton fullWidth /> </Box> ) : shouldUseMediaPreview ? (🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/feed-ui/src/ZoraCoinCreatedItem.tsx` around lines 44 - 47, The skeleton used while metadata is loading is missing the fullWidth prop causing a width jump between the metadata-loading state and the image-loading fallback; update the ImageSkeleton instance inside the conditional that checks shouldUseMediaPreview and displayImageUrl (the one wrapped by Box with className feedItemImage in ZoraCoinCreatedItem) to pass fullWidth so it matches the later <ImageSkeleton fullWidth /> used for the FallbackImage placeholder.packages/feed-ui/src/ZoraDropCreatedItem.tsx (1)
44-47:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInconsistent skeleton sizing during media type loading.
Line 46 uses
<ImageSkeleton />withoutfullWidth, while Line 64 uses<ImageSkeleton fullWidth />for the FallbackImage placeholder. This creates inconsistent placeholder sizing between the media type loading phase and the image loading phase, similar to the issue in ZoraCoinCreatedItem.🎨 Proposed fix for consistent sizing
{isMediaTypeLoading ? ( <Box className={feedItemImage}> - <ImageSkeleton /> + <ImageSkeleton fullWidth /> </Box> ) : shouldUseMediaPreview ? (🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/feed-ui/src/ZoraDropCreatedItem.tsx` around lines 44 - 47, The media-type loading skeleton is sized inconsistently because the isMediaTypeLoading branch renders <ImageSkeleton /> without the fullWidth prop while the FallbackImage uses <ImageSkeleton fullWidth />; update the isMediaTypeLoading rendering to pass fullWidth to ImageSkeleton so both placeholders use the same sizing (look for the isMediaTypeLoading conditional and the ImageSkeleton component usage in ZoraDropCreatedItem.tsx).
🧹 Nitpick comments (1)
packages/zord/src/elements/Button.css.ts (1)
29-31: ⚡ Quick winAvoid enabling default navigation for
Button disabledwhen polymorphing to links
Button.css.ts’s disabled state only setscursor: not-allowed(nopointer-events: none), andButton.tsxjust drops the ReactonClickhandler whendisabled. For polymorphic renders likeas="a"/as={Link}withhref, the browser default action can still fire becausedisabledis inert on anchors andFlex/Boxdon’t add click suppression.No
Buttonusages withdisabledwere found in this repo combined withas="a"/as={Link}andhrefon the same element, but the behavior would be problematic if that combination is introduced. Consider guarding default behavior inhandleClick(e.g.,preventDefaultwhendisabledand the underlying element supports native navigation) or restoringpointer-events: nonefor disabled non-buttonpolymorphs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/zord/src/elements/Button.css.ts` around lines 29 - 31, The disabled styling only sets cursor and Button.tsx removes the React onClick but doesn't prevent native navigation when the component is polymorphed to an anchor/link with href; update the disabled handling so native navigation can't occur: either add pointer-events: none for non-button polymorphs in Button.css.ts (the '&[disabled]' rule) or modify the Button.tsx click handler (e.g., handleClick) to call event.preventDefault() and return early when props.disabled and the rendered element supports navigation (detect via the polymorphic "as" prop or presence of href), ensuring anchors/Link components cannot navigate when disabled.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/ui/src/Artwork/ArtworkUpload.tsx`:
- Line 126: The current conditional can render a stray "0" because fileCount is
number|string and when it's 0 the && expression short-circuits to 0; change the
guard to explicitly test for a positive count (e.g. replace fileCount &&
traitCount > 0 with Number(fileCount) > 0 && traitCount > 0 && layerOrdering or
Boolean(Number(fileCount)) && traitCount > 0 && layerOrdering) so artworkError,
uploadError, fileCount, traitCount and layerOrdering are still used but no
literal 0 is rendered.
---
Outside diff comments:
In `@packages/feed-ui/src/ZoraCoinCreatedItem.tsx`:
- Around line 44-47: The skeleton used while metadata is loading is missing the
fullWidth prop causing a width jump between the metadata-loading state and the
image-loading fallback; update the ImageSkeleton instance inside the conditional
that checks shouldUseMediaPreview and displayImageUrl (the one wrapped by Box
with className feedItemImage in ZoraCoinCreatedItem) to pass fullWidth so it
matches the later <ImageSkeleton fullWidth /> used for the FallbackImage
placeholder.
In `@packages/feed-ui/src/ZoraDropCreatedItem.tsx`:
- Around line 44-47: The media-type loading skeleton is sized inconsistently
because the isMediaTypeLoading branch renders <ImageSkeleton /> without the
fullWidth prop while the FallbackImage uses <ImageSkeleton fullWidth />; update
the isMediaTypeLoading rendering to pass fullWidth to ImageSkeleton so both
placeholders use the same sizing (look for the isMediaTypeLoading conditional
and the ImageSkeleton component usage in ZoraDropCreatedItem.tsx).
In `@packages/zord/src/elements/Button.tsx`:
- Around line 118-146: The current useMemo for handleClick only returns
undefined when disabled, which doesn't stop native element behavior for
polymorphic elements (e.g., anchors); change handleClick so that when disabled
it returns an actual event handler that calls event.preventDefault() and
event.stopPropagation() (and does not call the original onClick), and when not
disabled returns the provided onClick; update the handler signature to accept
the MouseEvent used by the Button component so this safe-no-op prevents
navigation for Button elements rendered as anchors while preserving normal
behavior for real buttons.
---
Nitpick comments:
In `@packages/zord/src/elements/Button.css.ts`:
- Around line 29-31: The disabled styling only sets cursor and Button.tsx
removes the React onClick but doesn't prevent native navigation when the
component is polymorphed to an anchor/link with href; update the disabled
handling so native navigation can't occur: either add pointer-events: none for
non-button polymorphs in Button.css.ts (the '&[disabled]' rule) or modify the
Button.tsx click handler (e.g., handleClick) to call event.preventDefault() and
return early when props.disabled and the rendered element supports navigation
(detect via the polymorphic "as" prop or presence of href), ensuring
anchors/Link components cannot navigate when disabled.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 637fc58a-8c4b-4266-9413-12e6918bba52
📒 Files selected for processing (22)
apps/web/src/modules/dashboard/CreateActions.tsxpackages/create-dao-ui/src/components/AllocationForm/AllocationForm.tsxpackages/create-dao-ui/src/components/Artwork/ArtworkUpload.tsxpackages/create-dao-ui/src/components/ConfirmReset/ConfirmReset.tsxpackages/create-dao-ui/src/components/FormNavButtons/FormNavButtons.css.tspackages/create-dao-ui/src/components/FormNavButtons/FormNavButtons.tsxpackages/create-dao-ui/src/components/ReviewAndDeploy/ReviewSection.css.tspackages/create-dao-ui/src/components/ReviewAndDeploy/ReviewSection.tsxpackages/dao-ui/src/components/MinterManagementModal/MinterManagementModal.tsxpackages/feed-ui/src/AuctionBidPlacedItem.tsxpackages/feed-ui/src/AuctionCreatedItem.tsxpackages/feed-ui/src/AuctionSettledItem.tsxpackages/feed-ui/src/ClankerTokenCreatedItem.tsxpackages/feed-ui/src/FeedSkeleton.tsxpackages/feed-ui/src/ZoraCoinCreatedItem.tsxpackages/feed-ui/src/ZoraDropCreatedItem.tsxpackages/ui/src/Artwork/ArtworkUpload.tsxpackages/zord/src/elements/Button.css.tspackages/zord/src/elements/Button.tsxpackages/zord/src/theme.css.tspackages/zord/src/tokens/color.tspackages/zord/src/utils/color-theme.ts
💤 Files with no reviewable changes (2)
- packages/create-dao-ui/src/components/FormNavButtons/FormNavButtons.css.ts
- packages/create-dao-ui/src/components/ReviewAndDeploy/ReviewSection.css.ts
1 participant
Summary by CodeRabbit
New Features
Improvements
Bug Fixes