Skip to content
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

Edit NFTs [1/2]: Redo /settings/nfts to account for all project NFT states #4051

Merged
merged 7 commits into from
Sep 9, 2023

Conversation

johnnyd-eth
Copy link
Contributor

@johnnyd-eth johnnyd-eth commented Sep 1, 2023

Sorry this one's quite fat, this solves all the NFT states exclusively within settings/nfts:

  1. Edit exitsting collection (unchanged UI), reshuffle some components
  2. No NFTs, operator permissions not yet set
  3. No NFTs, operator permissions set -> add the collection (call reconfigure FC with NFTs)

Project already has NFTs (no UI changes):
Screen Shot 2023-09-04 at 7 56 44 pm

Project has no NFTs and operator permissions not set:

Kapture.2023-09-04.at.20.44.43.mp4

-> then, after automatic page reload once setOperatorPermission completes ->
Project has no NFTs, operator permissions are set:

Kapture.2023-09-04.at.20.51.46.mp4

@vercel
Copy link

vercel bot commented Sep 1, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Ignored Deployments
Name Status Preview Updated (UTC)
juice-interface ⬜️ Ignored (Inspect) Visit Preview Sep 9, 2023 5:40am
juice-interface-goerli ⬜️ Ignored (Inspect) Visit Preview Sep 9, 2023 5:40am

</span>
<Button type="primary" onClick={() => setEnableNftsModalOpen(true)}>
<span>
<Trans>Enable NFTs</Trans>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This button enables operator permissions (allow NFT contract reconfig FC)

@@ -65,111 +65,109 @@ export const useReconfigureFundingCycle = ({
const reconfigureV2V3FundingCycleWithNftsTx =
useReconfigureV2V3FundingCycleWithNftsTx()

const {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The diff went whack here but the only thing that's changed is:

  • Flesh out the editingFundingCycleConfig object inside reconfigureFundingCycle() rather than outside
  • Add optional latestEditingData?: EditingFundingCycleConfig to reconfigureFundingCycle()
  • Dependencies changed

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it, ty!

@johnnyd-eth johnnyd-eth force-pushed the launch-nfts-from-nfts-page branch from 0eae60f to 7900a13 Compare September 4, 2023 10:56
@johnnyd-eth johnnyd-eth changed the title Launch new NFT collection flow Redo /settings/nfts to account for all project NFT states Sep 4, 2023
@johnnyd-eth johnnyd-eth changed the title Redo /settings/nfts to account for all project NFT states Edit NFTs [1/2]: Redo /settings/nfts to account for all project NFT states Sep 8, 2023
@tomquirk
Copy link
Contributor

tomquirk commented Sep 8, 2023

error when editing collection for this proj. it uses the latest NFT contracts so it may be a bug unrelated to this PR

Screenshot 2023-09-08 at 8 57 21 PM

Copy link
Contributor

@tomquirk tomquirk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@johnnyd-eth tested and lgtm! great work here!

@@ -43,8 +44,8 @@ export function EnableNftsModal({
confirmLoading={loading}
>
<Trans>
To add NFTs to your cycle. You'll need to{' '}
<strong>grant NFT permissions</strong> before launching your new cycle.
To add NFTs to your next cycle, you'll need to{' '}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

praise nice catch

@@ -65,111 +65,109 @@ export const useReconfigureFundingCycle = ({
const reconfigureV2V3FundingCycleWithNftsTx =
useReconfigureV2V3FundingCycleWithNftsTx()

const {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it, ty!

@johnnyd-eth johnnyd-eth merged commit b6c93e7 into main Sep 9, 2023
4 checks passed
@johnnyd-eth johnnyd-eth deleted the launch-nfts-from-nfts-page branch September 9, 2023 05:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants