-
Notifications
You must be signed in to change notification settings - Fork 77
chore(v2): cleanup rescan, seed modal, fee config; add developer mode #974
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
2c05d0c
to
77564ee
Compare
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.
LGTM
miningFormRef.current && !miningFormRef.current.getFormData() ? 'border-red-300' : '' | ||
}`} | ||
<Accordion | ||
type="single" |
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.
type="single" | |
type="single" | |
collapsible |
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.
Thanks for the careful review. This has been done deliberately (mentioned in the PR description as "Note: Fee dialog not 100% working correctly. Needs to be fixed in a follow-up PR.") - as the inner forms use ForwardRef and do not work correctly if placed within the Accordion (previously was a custom made component).
I want to revisit that at a later stage, I think there is a lot of work to do regarding forms, especially validation and input of amounts.
Okay for you if it stays this way for now?
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.
That sounds good! I thought it was a small fix so suggested it in this pr, but didn't know that it was deliberate.
Okay for you if it stays this way for now?
ofcourse!
}`} | ||
<Accordion | ||
defaultValue="collaborator-fees" | ||
type="single" |
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.
The accordion isn't collapsing for both collaborator and mining fees form
the issue is Radix UI's Accordion component with type="single" doesn't allow collapsing by default. we need to explicitly set collapsible={true} (or just collapsible)
type="single" | |
type="single" | |
collapsible |
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.
Same as above.
still needs improvements, e.g. accordion not working, validation, etc.
@chromatic-com/storybook 4.0.0 → 4.0.1 @playwright/test 1.53.1 → 1.53.2 @radix-ui/react-accordion 1.2.11 → 1.2.12 @radix-ui/react-dialog 1.1.14 → 1.1.15 @radix-ui/react-select 2.2.5 → 2.2.6 @radix-ui/react-tooltip 1.2.7 → 1.2.8 @storybook/addon-a11y 9.0.12 → 9.0.18 @storybook/addon-docs 9.0.12 → 9.0.18 @storybook/addon-vitest 9.0.12 → 9.0.18 @storybook/react-vite 9.0.12 → 9.0.18 @tailwindcss/vite 4.1.10 → 4.1.13 @tanstack/react-query 5.87.1 → 5.87.4 @types/node 22.15.29 → 22.15.35 @types/react 19.1.8 → 19.1.13 @types/react-dom 19.1.6 → 19.1.9 conventional-changelog 7.1.0 → 7.1.1 eslint-plugin-storybook 9.0.12 → 9.0.18 husky 8.0.0 → 8.0.3 lint-staged 16.1.2 → 16.1.6 prettier-plugin-tailwindcss 0.6.13 → 0.6.14 react 19.1.0 → 19.1.1 react-dom 19.1.0 → 19.1.1 react-router-dom 7.6.2 → 7.6.3 sonner 2.0.5 → 2.0.7 storybook 9.0.12 → 9.0.18 tailwindcss 4.1.10 → 4.1.13 tw-animate-css 1.3.4 → 1.3.8 vite 6.3.5 → 6.3.6 zustand 5.0.7 → 5.0.8 @tanstack/react-query 5.87.4 → 5.89.0 @types/node 22.15.35 → 22.18.6 @vitejs/plugin-react 4.5.2 → 4.7.0 eslint 9.29.0 → 9.35.0 eslint-plugin-storybook 9.0.18 → 9.1.7 globals 16.2.0 → 16.4.0 lucide-react 0.518.0 → 0.544.0 prettier 3.5.3 → 3.6.2 react-i18next 15.5.3 → 15.7.3 react-router-dom 7.6.3 → 7.9.1 typescript ~5.8.3 → ~5.9.2 typescript-eslint 8.34.1 → 8.44.0
75f4227
to
f58e585
Compare
Note: Fee dialog not 100% working correctly. Needs to be fixed in a follow-up PR.