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

enable SyncCreatorTokens for Existing Contracts #2003

Merged
merged 22 commits into from
Oct 25, 2023

Conversation

Rohan-cp
Copy link
Collaborator

@Rohan-cp Rohan-cp commented Oct 9, 2023

Summary of Changes

You can refresh collections of created tokens from
-web: NftSelector, Editor
-mobile: NftSelector

Demo Mobile

newehoqeoq.mov

Demo Web

Screen.Recording.2023-10-18.at.4.29.18.PM.MOV
Screen.Recording.2023-10-18.at.4.31.17.PM.MOV

Edge Cases

List any common edge cases that you have considered and tested.

Testing Steps

Can test locally with a creator

Checklist

Please make sure to review and check all of the following:

  • I've tested the changes and all tests pass.
  • (if web) I've tested the changes on various desktop screen sizes to ensure responsiveness.
  • (if mobile) I've tested the changes on both light and dark modes.

@vercel
Copy link

vercel bot commented Oct 9, 2023

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

Name Status Preview Comments Updated (UTC)
gallery ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 23, 2023 11:16pm

@github-actions github-actions bot added the web label Oct 9, 2023
@github-actions
Copy link

github-actions bot commented Oct 9, 2023

Bundle Sizes

Compared against 67b71c6

Route Size (gzipped) Diff
/[username] 346.17 KB +1.42 KB
/[username]/[collectionId] 351.98 KB +1.42 KB
/[username]/[collectionId]/[tokenId] 586.25 KB +2.43 KB
/[username]/followers 350.59 KB +1.42 KB
/[username]/galleries 370.71 KB +1.42 KB
/[username]/galleries/[galleryId] 348.98 KB +1.42 KB
/[username]/posts 715.68 KB +1.42 KB
/[username]/token/[tokenId] 583.92 KB +2.43 KB
/community/[chain]/[contractAddress] 700.83 KB +1.51 KB
/community/[chain]/[contractAddress]/live 710.19 KB +1.51 KB
/explore 343.87 KB +1.42 KB
/following 685.94 KB +1.42 KB
/gallery/[galleryId]/edit 295.3 KB +2.61 KB
/home 685.5 KB +1.42 KB
/latest 685.36 KB +1.42 KB
/mobile 314.08 KB +1.42 KB
/post/[postId] 377.37 KB +1.42 KB
Dynamic import Size (gzipped) Diff
../src/contexts/AppProvider.tsx -> ./fullPageNftDetailModalListener/FullPageNftDetailModalListener 514.17 KB +2.41 KB
../src/contexts/relay/network.ts -> persisted_queries.json 73.25 KB +1.72 KB

@Rohan-cp Rohan-cp requested review from Robinnnnn and kaitoo1 October 18, 2023 10:30
@Rohan-cp Rohan-cp marked this pull request as ready for review October 18, 2023 10:31
@Rohan-cp Rohan-cp changed the title enable Contract/Collection refresh for Creators enable SyncCreatorTokens for Existing Contracts Oct 18, 2023
Copy link
Contributor

@Robinnnnn Robinnnnn left a comment

Choose a reason for hiding this comment

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

going over this in loom but just to reiterate:

  • let's make sure the mobile refresh icon is more accurate in its reaction
  • let's share a generic RefreshIcon component
  • some tracking updates
  • deprecate setCollectionContractId and use existing state vehicles

const track = useTrack();
const handleCreatorRefreshContract = useCallback(
async (contractId: string) => {
track('Editor Sidebar: Clicked Sync Creator Tokens For Existing Contract');
Copy link
Contributor

Choose a reason for hiding this comment

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

let's make sure to fix the tracking:

track('Editor Sidebar: Clicked Sync Creator Tokens For Existing Contract', {
  id: 'Refresh Single Created Contract Button',
  context: contexts.Editor,
})

Comment on lines 73 to 80
selectedView={selectedView}
setSpamPreferenceForCollection={setSpamPreferenceForCollection}
/>
) : shouldDisplayContractRefreshIcon ? (
<RefreshContractIcon contractId={row.contractId} />
) : (
<CollectionCount>{row.count}</CollectionCount>
)}
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: i think there's a lot of logic going on here, could simplify to:

  const rightContent = useMemo(() => {
    if (selectedView === 'Created' && isMouseHovering) {
      return <RefreshContractIcon contractId={row.contractId} />;
    }
    if (selectedView !== 'Created' && isMouseHovering) {
      return (
        <ToggleSpamIcon
          row={row}
          selectedView={selectedView}
          setSpamPreferenceForCollection={setSpamPreferenceForCollection}
        />
      );
    }
    return <CollectionCount>{row.count}</CollectionCount>;
  }, [isMouseHovering, row, selectedView, setSpamPreferenceForCollection]);

...


// render
{rightContent}

};

function AnimatedRefreshContractIcon({ contractId, onRefresh }: AnimatedRefreshContractIconProps) {
const { isSyncing, syncCreatedTokensForExistingContract } = useSyncTokenstActions();
Copy link
Contributor

Choose a reason for hiding this comment

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

typo

}
eventElementId="NftSelectorSyncCreatedTokensForContractButton"
eventName="Nft Selector SyncCreatedTokensForContractButton pressed"
eventContext={contexts.UserCollection}
Copy link
Contributor

Choose a reason for hiding this comment

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

contexts.Posts

if (!collectionContractId) {
return;
}
track('NFT Selector: Clicked Sync Creator Tokens For Existing Contract');
Copy link
Contributor

Choose a reason for hiding this comment

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

same thing, let's make sure to track additional details: { id: Refresh Single Contract Button, context: contexts.Posts }

@Robinnnnn Robinnnnn merged commit 5255e6f into main Oct 25, 2023
@Robinnnnn Robinnnnn deleted the rohan/creator-refresh-collection branch October 25, 2023 00:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants