[2/2] Cleanup Create/Settings NFT tech debt #4066
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
johnnyd-eth
changed the title
…FT form and related components
johnnyd-eth
force-pushed
the
create-settings-nft-tech-debt
branch
from
c69ec89 to
2b5e79d
Compare
| /> | ||
| } | ||
| /> | ||
| <TransactionModal transactionPending open={launchTxPending} /> |
There was a problem hiding this comment.
Move these modals from the button component into the page component
| }) { | ||
| const [ipfsUploading, setIpfsUploading] = useState<boolean>(false) |
There was a problem hiding this comment.
Move all this shite into the new settings launch NFTs hook
johnnyd-eth
changed the title
johnnyd-eth
force-pushed
the
create-settings-nft-tech-debt
branch
from
2882754 to
b613d22
Compare
| import { CreateCollapse } from '../../Create/components/CreateCollapse' | ||
| import { OptionalHeader } from '../../Create/components/OptionalHeader' |
There was a problem hiding this comment.
suggestion if these aren't create-flow specific, might we move them out to the parent dir?
There was a problem hiding this comment.
Yes we should, was just keeping it mission critical for now - figured this would explode the diff. A tech-debt task for the future for sure
| } | ||
| /> | ||
| </> | ||
| <Button |
There was a problem hiding this comment.
sugestion: given this component now does nothing specific (its just a button!) is it worth even having? could we nuke?
| }> | ||
|
|
||
| export const useNftRewardsForm = () => { | ||
| export const useCreateFlowNftRewardsForm = () => { |
There was a problem hiding this comment.
suggestion (non-blocking) maybe this could be called "useNewProjectNftRewardsForm`
| } from '../../../ReconfigureFundingCycleSettingsPage/hooks/useEditingFundingCycleConfig' | ||
| import { useReconfigureFundingCycle } from '../../../ReconfigureFundingCycleSettingsPage/hooks/useReconfigureFundingCycle' | ||
|
|
||
| export const useSettingsLaunchNftsForm = () => { |
There was a problem hiding this comment.
suggestion maybe simply useLaunchNftsForm
There was a problem hiding this comment.
@johnnyd-eth fantastic clean up PR, thanks for a great description as well to orient me. Approved!
johnnyd-eth
force-pushed
the
create-settings-nft-tech-debt
branch
from
b613d22 to
fb73c0e
Compare
The motivation for this PR was to fix a few bugs caused by the linkage of the
AddNftCollectionFormacross both settings and create:Moves
AddNftCollectionFormand any related components to the sharedNftRewardsdirectoryMoves create flow logic out of
AddNftCollectionForm(shared component) into newNftRewardsPage(create flow specific component)Changes name of
useNftRewardsFormhook touseCreateFlowNftRewardsFormCreates new
useSettingsLaunchNftsFormhook which relies purely on the ant form rather than redux (as the create hook does)Moves all logic inside the
UploadAndLaunchNftsButtoninto theuseSettingsLaunchNftsForm