-
Notifications
You must be signed in to change notification settings - Fork 619
[Dashboard] Feature: Modules UI changes #5437
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
|
Your org has enabled the Graphite merge queue for merging into mainAdd the label “merge-queue” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix. You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link. |
...oard/src/app/(dashboard)/(chain)/[chain_id]/[contractAddress]/modules/components/Royalty.tsx
Outdated
Show resolved
Hide resolved
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5437 +/- ##
=======================================
Coverage 43.84% 43.84%
=======================================
Files 1081 1081
Lines 56186 56186
Branches 3929 3929
=======================================
Hits 24635 24635
Misses 30868 30868
Partials 683 683
*This pull request uses carry forward flags. Click here to find out more. |
size-limit report 📦
|
f57f876 to
cf98aa5
Compare
|
Linear ticket please |
a40de04 to
407880a
Compare
...nents/contract-components/contract-deploy-form/modular-contract-default-modules-fieldset.tsx
Show resolved
Hide resolved
...ard/src/components/contract-components/contract-deploy-form/sequential-token-id-fieldset.tsx
Outdated
Show resolved
Hide resolved
...oard/src/app/(dashboard)/(chain)/[chain_id]/[contractAddress]/modules/components/Royalty.tsx
Outdated
Show resolved
Hide resolved
...oard/src/app/(dashboard)/(chain)/[chain_id]/[contractAddress]/modules/components/Royalty.tsx
Outdated
Show resolved
Hide resolved
| percentage: z | ||
| .string() | ||
| .min(1, { message: "Invalid BPS" }) | ||
| .refine((v) => Number(v) >= 0, { message: "Invalid BPS" }), | ||
| .min(1, { message: "Invalid percentage" }) | ||
| .refine((v) => Number(v) === 0 || (Number(v) >= 0.01 && Number(v) <= 100), { | ||
| message: "Invalid percentage", | ||
| }), |
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.
There appears to be conflicting validation logic for zero values. The min(1) string validation will reject "0" inputs, while the refine() check explicitly allows zero values with Number(v) === 0. To consistently support zero values, the min(1) validation should be removed, leaving only the refine() check to handle the full range of valid inputs (0 and 0.01-100).
Spotted by Graphite Reviewer
Is this helpful? React 👍 or 👎 to let us know.
|
|
||
| <FormErrorMessage>{props.errorMessage}</FormErrorMessage> | ||
|
|
||
| <FormHelperText className="!text-sm text-muted-foreground"> |
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.
If you are using FormFieldSetup -
you can remove FormLabel, FormErrorMessage and FormHelperText and just pass those to props label , errorMessage, helperText etc to avoid adding chakra components
Merge activity
|
https://linear.app/thirdweb/issue/DASH-469/small-changes-for-modules-ui <!-- start pr-codex --> --- ## PR-Codex overview This PR focuses on enhancing the functionality and user experience of the `Transferable`, `Claimable`, and `Royalty` components in the dashboard application, particularly by adding support for ERC20 tokens and improving form handling for royalties. ### Detailed summary - Added support for `isErc20` state in `Transferable` and `Claimable` components. - Updated UI to display messages related to transfer restrictions and accounts. - Introduced `SequentialTokenIdFieldset` to handle token ID inputs. - Changed royalty handling from BPS to percentage in `Royalty` component. - Improved form validation and error messaging for royalty inputs. > ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your question}` <!-- end pr-codex -->
e83056b to
a98550d
Compare
https://linear.app/thirdweb/issue/DASH-469/small-changes-for-modules-ui
PR-Codex overview
This PR primarily focuses on enhancing the
Transferable,Claimable, andRoyaltycomponents by adding new features, improving user interfaces, and correcting data handling related to ERC20 and ERC721 tokens.Detailed summary
isErc20state inClaimableandRoyaltycomponents.SequentialTokenIdFieldsetfor handling token ID inputs.bpstopercentageinRoyaltycalculations and UI.