feat: Create Collection Atomic Swap #11410
Conversation
Jarsen136
requested review from
preschian and
vikiival
and removed request for
a team
✅ Deploy Preview for polkadot ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for koda-canary ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Deploying koda-art-prod with
|
| Latest commit: |
424df7d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://a65dec5d.kodaart-production.pages.dev |
| Branch Preview URL: | https://issue-11331-0.kodaart-production.pages.dev |
components/swap/PreviewItem.vue
Outdated
| @@ -22,6 +22,7 @@ | |||
| </div> | |||
|
|
|||
| <NeoButton | |||
| v-if="!hideRemove" | |||
There was a problem hiding this comment.
where is hideRemove coming from?
WalkthroughThis update introduces a collection swap feature across various components. It standardizes prop names, refactors trade action logic using a new composable, and adjusts routing and middleware to support collection swaps. New swap components, pages, and localized strings are added, and type definitions for swap items are updated to allow null values. Overall, the changes streamline the UI and control flow for trade and swap operations. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant CSB as CreateCollectionSwapButton
participant TS as TradeActionClick Composable
participant SwapStore as Swap Store
participant Router as Router
U->>CSB: Click on Create Collection Swap
CSB->>TS: Call onCreateCollectionSwapClick()
TS->>SwapStore: createSwap(collectionId, config)
SwapStore-->>TS: Return new swap
TS->>Router: navigateToSwap(newSwap)
Router-->>U: Redirect to swap interface
sequenceDiagram
participant U as User
participant TACC as useTradeActionClick
participant Identity as IdentityStore
participant Chain as ChainChecker
U->>TACC: Trigger onTradeActionClick(callback)
alt User is owner
TACC-->>U: Execute callback immediately
else User is not owner
alt User is logged in
TACC->>Chain: doAfterCheckCurrentChainVM(callback)
Chain-->>TACC: Chain verified
TACC-->>U: Execute callback
else Not logged in
TACC->>Identity: doAfterLogin(callback)
Identity-->>TACC: Login success
TACC-->>U: Execute callback
end
end
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
middleware/swap.ts (1)
24-33: Simplify navigation logic with early returns.The nested if conditions could be simplified for better readability and maintainability.
if (!foundSwap) { - if (isAddress(id)) { - return navigateTo({ - name: getSwapStepRouteName(SwapStep.DESIRED), - params: { id, prefix }, - query: { swapId: swapStore.createSwap(id).id }, - }) - } - return navigateTo({ name: getSwapStepRouteName(SwapStep.COUNTERPARTY, true), params: { id, prefix } }) + if (!isAddress(id)) { + return navigateTo({ + name: getSwapStepRouteName(SwapStep.COUNTERPARTY, true), + params: { id, prefix } + }) + } + + return navigateTo({ + name: getSwapStepRouteName(SwapStep.DESIRED), + params: { id, prefix }, + query: { swapId: swapStore.createSwap(id).id }, + }) }utils/swap.ts (1)
89-92: Update or remove the outdated comment.The comment "collection swaps are not supported yet" is now outdated as collection swaps are being implemented in this PR.
- // collection swaps are not supported yet + // Skip counter swap for collection trades
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
components/collection/CollectionTrades.vue(1 hunks)components/collection/drop/ItemsGrid.vue(1 hunks)components/gallery/GalleryItemAction/GalleryItemActionType/GalleryItemTrade.vue(1 hunks)components/items/ItemsGrid/ItemsGrid.vue(3 hunks)components/items/ItemsGrid/ItemsGridImage.vue(2 hunks)components/shared/nftCard/NftCard.vue(2 hunks)components/shared/nftCard/NftMediaInfo.vue(3 hunks)components/swap/CreateCollectionSwapButton.vue(1 hunks)components/swap/Preview.vue(4 hunks)components/swap/PreviewItem.vue(2 hunks)components/swap/banner/CollectionPreview.vue(1 hunks)components/swap/banner/accounts.vue(3 hunks)components/swap/collection/DestinationProfile.vue(1 hunks)components/swap/collection/SwapGridList.vue(1 hunks)components/swap/layout/index.vue(1 hunks)components/swap/review.vue(4 hunks)components/trade/TradeActivityTable.vue(1 hunks)composables/transaction/transactionCreateSwap.ts(1 hunks)composables/transaction/types.ts(1 hunks)composables/useTradeActionClick.ts(1 hunks)i18n/locales/en.json(1 hunks)middleware/redirects.global.ts(2 hunks)middleware/swap.ts(2 hunks)pages/[prefix]/swap/collection/[id]/index.vue(1 hunks)stores/atomicSwaps.ts(2 hunks)utils/swap.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- components/trade/TradeActivityTable.vue
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: shard (10, 10)
- GitHub Check: build-cf
- GitHub Check: test
- GitHub Check: runner / eslint
- GitHub Check: build
- GitHub Check: Cloudflare Pages: koda-art-prod
🔇 Additional comments (37)
pages/[prefix]/swap/collection/[id]/index.vue (2)
1-3: LGTM! Clean and focused template.The template follows a single responsibility principle by delegating the rendering to a dedicated component.
5-10: LGTM! Proper page configuration.The page meta configuration appropriately sets up:
- Background layout for consistent UI
- Swap middleware for validation
components/swap/layout/index.vue (1)
9-9: LGTM! Proper prop binding.The is-collection-swap prop is correctly bound to the swap store's state.
composables/useTradeActionClick.ts (2)
4-9: LGTM! Well-structured composable setup.The composable properly:
- Takes isOwner as a computed ref
- Sets up identity store access
- Initializes login helper
17-23: LGTM! Proper authentication flow.The login and chain verification flow is well-structured:
- Checks login state
- Handles chain verification for logged-in users
- Manages login flow for non-authenticated users
components/swap/collection/DestinationProfile.vue (2)
1-19: LGTM! Well-structured template with clear component hierarchy.The template effectively:
- Uses layout component for consistent structure
- Shows progress with step indicator
- Implements grid list for collection items
- Includes preview section
28-34: Consider adding error handling for edge cases.While the query filtering is correct, consider handling these edge cases:
- When collectionId is invalid/missing
- When accountId is not available
Add validation and error states:
const query = reactive({ collection: { - id_eq: collectionId.value, + id_eq: collectionId.value || '', }, - currentOwner_not_eq: accountId.value, + currentOwner_not_eq: accountId.value || '', burned_eq: false, }) + +// Add error handling +const hasError = computed(() => !collectionId.value || !accountId.value) +watch(hasError, (error) => { + if (error) { + // Handle error state + } +})components/swap/PreviewItem.vue (2)
24-30: LGTM! Conditional rendering of remove button.The implementation correctly uses
v-ifdirective to conditionally render the remove button based on theremovableprop.
39-49: LGTM! Props definition with defaults.The use of
withDefaultsto define props with TypeScript is correct and follows Vue.js best practices.components/swap/collection/SwapGridList.vue (2)
11-21: Add error handling for undefined NFTs.Consider adding error handling for cases where NFTs might be undefined or have missing properties.
v-for="(nft, index) in desiredNfts ?? []"
35-38: LGTM! Store and computed properties setup.The implementation correctly uses Vue's Composition API with TypeScript for store management and computed properties.
components/swap/banner/accounts.vue (2)
25-33: LGTM! Conditional rendering of collection preview.The implementation correctly handles both collection and address display cases.
Note: As discussed in past reviews, the counterparty section remains until a new design is provided (issue #11425).
41-45: LGTM! Props definition.The TypeScript props definition is correct and includes the new optional
isCollectionSwapboolean.components/swap/CreateCollectionSwapButton.vue (1)
22-26: Add error handling for undefined collection.The computed property should handle cases where the collection might be undefined.
-const isCollectionOwner = computed(() => isCurrentAccount(collection.value.currentOwner)) +const isCollectionOwner = computed(() => collection.value?.currentOwner ? isCurrentAccount(collection.value.currentOwner) : false)components/collection/drop/ItemsGrid.vue (1)
29-29: LGTM! Improved prop naming convention.The rename from
collection-popover-hidetohide-collection-popoverfollows Vue's recommended naming convention for boolean props, making the prop's purpose more intuitive.composables/transaction/transactionCreateSwap.ts (1)
21-21: LGTM! Enhanced type safety for blockchain transaction.The explicit type casting of
offeredItemto string ensures type safety when interacting with the Polkadot API'screateSwapfunction.middleware/swap.ts (1)
34-34:❓ Verification inconclusive
Consider adding type assertion for isCollectionSwap.
The
isCollectionSwapvalue is derived fromfoundSwapbut its type safety isn't guaranteed.
🏁 Script executed:
#!/bin/bash # Check if isCollectionSwap is properly typed in the codebase ast-grep --pattern 'interface $_ { $$$ isCollectionSwap: boolean $$$ }'Length of output: 79
Type Safety Check for
isCollectionSwapThe assignment to
isCollectionSwap(viaconst isCollectionSwap = foundSwap.isCollectionSwap) currently lacks explicit type assurance. Please verify that the type offoundSwapexplicitly definesisCollectionSwapas a boolean. If not, consider adding a type assertion or updating the type definition accordingly to ensure type safety and prevent potential runtime issues.components/collection/CollectionTrades.vue (1)
15-19: LGTM! Well-structured implementation of the collection swap action.The addition of the action slot with conditional rendering of the CreateCollectionSwapButton follows component composition best practices, making the code modular and maintainable.
components/shared/nftCard/NftMediaInfo.vue (1)
7-7: LGTM! Improved prop naming convention.The renaming from
collectionPopoverHidetohideCollectionPopoverfollows Vue's best practices for boolean props, making the code more intuitive and maintainable.Also applies to: 20-20, 77-77
components/gallery/GalleryItemAction/GalleryItemActionType/GalleryItemTrade.vue (1)
53-55: LGTM! Excellent refactoring of trade action handling.Moving the login state handling to the
useTradeActionClickcomposable improves code organization, reduces complexity, and makes the component more focused on its core responsibility.utils/swap.ts (1)
12-17: LGTM! Well-structured implementation of collection swap routing.The addition of
COLLECTION_SWAP_ROUTE_NAME_STEP_MAPand the modifications togetSwapStepRouteNamefollow the existing pattern while cleanly extending support for collection swaps.Also applies to: 26-32
stores/atomicSwaps.ts (2)
21-21: LGTM! Type changes support collection swaps.The type changes appropriately support collection swaps by:
- Making
idandsnnullable for collection-level swaps- Adding
isCollectionSwapflag to differentiate swap typesAlso applies to: 25-25, 28-28
66-66: LGTM! Default to NFT swaps.Initializing
isCollectionSwaptofalsemakes NFT swaps the default behavior, which is a safe choice.middleware/redirects.global.ts (1)
3-3: LGTM! Added address validation.The addition of
isAddressvalidation improves route handling by preventing invalid addresses from being processed.Also applies to: 19-21
components/swap/review.vue (2)
58-63: LGTM! Conditional rendering based on swap type.The template correctly handles both collection and NFT swaps by conditionally rendering the appropriate grid list component.
118-118: LGTM! Routing logic handles collection swaps.The routing logic properly handles collection swaps by:
- Using SwapStep enum for type safety
- Passing collection swap context to route generation
Also applies to: 139-141
components/shared/nftCard/NftCard.vue (1)
62-62: LGTM! Improved prop naming convention.The prop rename from
collectionPopoverHidetohideCollectionPopoverfollows a more consistent verb-noun naming pattern.Also applies to: 128-128
components/items/ItemsGrid/ItemsGridImage.vue (1)
11-11: LGTM! Improved prop naming.The rename from
collectionPopoverHidetohideCollectionPopoverfollows better naming conventions and improves code clarity.components/swap/Preview.vue (3)
26-31: Prevent clearing items in collection swap desired step.The clear all button is now hidden for collection swap desired step, which is a good UX decision as it prevents accidental clearing of collection items.
49-49: Prevent removing items in collection swap when id is null.The item removal is disabled when
idis null in a collection swap, which is correct as these items represent collection placeholders.
148-165: Improved routing logic for collection swaps.The step details map now dynamically adapts based on the swap type, ensuring correct navigation paths for both regular and collection swaps.
components/items/ItemsGrid/ItemsGrid.vue (1)
29-29: LGTM! Consistent prop naming.The prop rename is consistently applied across the component, improving code clarity and maintainability.
Also applies to: 151-151, 287-287
composables/transaction/types.ts (1)
193-195:❓ Verification inconclusive
Verify null handling for collection swaps.
The
TokenToSwaptype now allows null values foridandsnto support collection swaps. Please ensure that components consuming this type properly handle these null values to prevent runtime errors.Run the following script to check for potential null handling issues:
🏁 Script executed:
#!/bin/bash # Description: Check for proper null handling of TokenToSwap properties # Search for direct access to id and sn properties without null checks rg -A 5 "swap\.(id|sn)(?!\?)" # Search for potential TypeScript non-null assertions rg -A 5 "swap\.(id|sn)!"Length of output: 294
Below is a revised review comment including a new verification script that uses the
--pcre2flag to properly support look-ahead. Please run the new script and verify the output manually to ensure allTokenToSwapconsumers handlenullvalues foridandsnsafely.
Review: Verify Proper Null Handling for Collection Swaps
The
TokenToSwaptype now acceptsnullvalues foridandsnto support collection swaps. It is critical that any component consuming these properties correctly checks fornullvalues to avoid runtime errors.To recheck for potential issues after encountering a regex error with the original script, please execute the updated script below (which uses the
--pcre2flag):Action Items:
- Ensure the components that read
swap.idandswap.sninclude null checks.- Review any matches or warnings from the above searches to confirm proper null handling is in place.
Please verify these changes manually, as the initial command output was inconclusive due to unsupported regex look-around.
🏁 Script executed:
[run_scripts] #!/bin/bash # Description: Check for potential null handling issues for TokenToSwap properties using PCRE2. echo "Searching for direct accesses without null checks (using --pcre2)..." rg --pcre2 -A 5 'swap\.(id|sn)(?!\?)' echo "Searching for non-null assertions (using --pcre2)..." rg --pcre2 -A 5 'swap\.(id|sn)!'Length of output: 648
Action: Verify Null Handling for TokenToSwap in utils/swap.ts
The updated verification script identified that in utils/swap.ts there's a direct access:
query: { swapId: swap.id },Since
TokenToSwap.idnow permitsnull(to support collection swaps), please review this usage to ensure it’s safe. Confirm that downstream consumers or query builders correctly account for potential null values—either by validating the value beforehand or by handling a null in the query logic. No non-null assertions were detected, so ensure that passing a null here is intentional and won’t lead to runtime errors.i18n/locales/en.json (4)
1841-1841: Localization Update: anyNftFromCollectionThe new key is defined as
"anyNftFromCollection": "Any NFT from \"{0}\" Collection", and the use of the placeholder{0}is appropriate for dynamic collection names. This clearly conveys the intended message for collection swap functionality.
1845-1845: Localization Update: collectionSwapDescriptionThe added key
"collectionSwapDescription": "Any NFT from this collection will be offered in the swap"is clear and concise, ensuring that users understand that the swap offer includes any NFT from the designated collection.
1846-1846: Localization Update: collectionSwapPreviewThe new key
"collectionSwapPreview": "Collection Swap"provides an intuitive label for the preview section in the swap UI. It is succinct and consistent with other action labels.
1851-1851: Localization Update: createCollectionSwapThe key
"createCollectionSwap": "Create Collection Swap"serves effectively as the call-to-action for initiating a collection swap. It aligns with the overall feature objective and the new UI flow.
feat: Create Collection Atomic Offer
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
components/collection/CollectionTrades.vue (1)
47-47: 🛠️ Refactor suggestionImprove type safety for route params.
Converting route params directly to string could be unsafe if the param is undefined.
-const collectionId = computed(() => route.params.id.toString()) +const collectionId = computed(() => { + const id = route.params.id + if (!id) { + throw new Error('Collection ID is required') + } + return id.toString() +})
🧹 Nitpick comments (5)
components/trade/types.ts (1)
8-91: Add JSDoc comments to improve type documentation.The types are well-organized, but adding JSDoc comments would improve maintainability by:
- Explaining the purpose of each type
- Documenting the relationship between types
- Clarifying when null values are expected
Example improvement:
+/** + * Represents an item being offered in a trade. + * When used in collection swaps, id and sn may be null. + */ export type MakingOfferItem = { urlPrefix: string // ... rest of the type }composables/useTradeActionClick.ts (1)
10-23: Add TypeScript types for better maintainability.The implementation is clean, but could benefit from explicit TypeScript types.
+type TradeActionCallback = () => void; +type LoginSuccessHandler = { onLoginSuccess: () => void }; -const onTradeActionClick = (cb: () => void) => { +const onTradeActionClick = (cb: TradeActionCallback) => { const fn = () => { if (!disabled?.value) { cb() } } if (isLogIn.value) { doAfterCheckCurrentChainVM(fn) } else { - doAfterLogin({ onLoginSuccess: fn }) + doAfterLogin({ onLoginSuccess: fn } as LoginSuccessHandler) } }components/collection/CollectionTrades.vue (1)
50-66: Simplify trade query construction.The query construction logic could be simplified using template literals and object literals.
const getTradeQuery = (direction: 'incoming' | 'outgoing') => { - const directionQuery = { - incoming: `considered: { id_eq: "${collectionId.value}" }`, - outgoing: `nft: { collection: { id_eq: "${collectionId.value}" } }`, - }[direction] + const directionQueries = { + incoming: (id: string) => `considered: { id_eq: "${id}" }`, + outgoing: (id: string) => `nft: { collection: { id_eq: "${id}" } }`, + } - const where = [ - 'status_eq: ACTIVE', - directionQuery, - ] - - if (tradeCollection.value) { - where.push(`desired_isNull: true`) - } - - return `{ ${where.join(', ')} }` + return `{ + status_eq: ACTIVE, + ${directionQueries[direction](collectionId.value)} + ${tradeCollection.value ? ', desired_isNull: true' : ''} + }` }components/trade/makeOffer/MakeOfferSingleItem.vue (2)
66-68: Make prop types more explicit.The optional boolean prop could benefit from a default value and more explicit typing.
const props = defineProps<{ offerPrice?: number - showPrice?: boolean + showPrice: boolean }>() + +withDefaults(defineProps<{ + offerPrice?: number + showPrice: boolean +}>(), { + showPrice: true +})
91-103: Simplify watch handlers using watchEffect.The watch handlers could be simplified using watchEffect for better reactivity handling.
-watch( - () => props.offerPrice, - value => emit('setOfferPrice', value), -) - -watch( - () => item.value?.price, - (value) => { - if (value) { - offerPrice.value = Number(value) - } - }, -) +watchEffect(() => { + if (props.offerPrice !== undefined) { + emit('setOfferPrice', props.offerPrice) + } +}) + +watchEffect(() => { + const price = item.value?.price + if (price) { + offerPrice.value = Number(price) + } +})
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
components/collection/CollectionTrades.vue(1 hunks)components/swap/CreateCollectionSwapButton.vue(1 hunks)components/trade/makeOffer/CreateCollectionOfferButton.vue(1 hunks)components/trade/makeOffer/MakeOfferModal.vue(1 hunks)components/trade/makeOffer/MakeOfferSingleItem.vue(2 hunks)components/trade/types.ts(1 hunks)composables/useTradeActionClick.ts(1 hunks)i18n/locales/en.json(2 hunks)queries/subsquid/general/collectionByIdMinimalWithRoyalty.graphql(1 hunks)queries/subsquid/general/highestOfferByCollectionId.graphql(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (19)
- GitHub Check: Redirect rules
- GitHub Check: Header rules
- GitHub Check: Redirect rules
- GitHub Check: Pages changed
- GitHub Check: Header rules
- GitHub Check: Pages changed
- GitHub Check: shard (10, 10)
- GitHub Check: shard (9, 10)
- GitHub Check: shard (8, 10)
- GitHub Check: shard (7, 10)
- GitHub Check: shard (6, 10)
- GitHub Check: shard (5, 10)
- GitHub Check: shard (4, 10)
- GitHub Check: shard (3, 10)
- GitHub Check: build-cf
- GitHub Check: build
- GitHub Check: shard (2, 10)
- GitHub Check: shard (1, 10)
- GitHub Check: Cloudflare Pages: koda-art-prod
🔇 Additional comments (9)
components/trade/types.ts (1)
14-14:❓ Verification inconclusive
LGTM! Verify null handling in consuming code.
Making
idandsnnullable aligns with the collection atomic swap feature. However, since this is a breaking change, ensure that:
- All consuming code properly handles null cases
- The inconsistency with
TradeTokentype (where these fields are required) is intentionalRun this script to find potential issues:
Also applies to: 20-20
🏁 Script executed:
#!/bin/bash # Description: Find usages of MakingOfferItem that might need updates # Find direct usages of MakingOfferItem echo "=== Direct usages of MakingOfferItem ===" rg "MakingOfferItem" -A 5 # Find places where id or sn are accessed without null checks echo -e "\n=== Potential missing null checks ===" ast-grep --pattern 'makingOfferItem.$_' | rg -e "\.id" -e "\.sn"Length of output: 5425
Attention: Confirm null handling in consuming code
Making
idandsnnullable in theMakingOfferItemtype supports the collection atomic swap feature as intended. However, since this change is a breaking one and creates an inconsistency with theTradeTokentype (where these fields remain non-nullable), please manually verify that:
- All usages of
MakingOfferItem—particularly in components likeCartItemDetails.vue,shoppingCart/utils.ts, and trade offer components—handle potentialnullvalues appropriately.- The transformation in functions (e.g.,
nftToOfferItem) safely assumes that values likenft.idare defined or properly guarded againstnull.- The discrepancy with
TradeTokenis deliberate and won’t lead to runtime errors or unexpected behavior in downstream code.Automated scans did not reveal any obvious missing null checks, but given the low output quality from the script, a thorough manual review is recommended.
composables/useTradeActionClick.ts (3)
8-8: LGTM! Clean and efficient login status check.The computed property efficiently derives the login status from the auth address with proper type conversion.
25-28: LGTM! Clean composable export.The return statement follows the composable pattern with a clear and descriptive function name.
1-4:❓ Verification inconclusive
Verify the impact of removing owner checks.
The function signature has changed from requiring an
isOwnerparameter to an optionaldisabledparameter. While this makes the composable more flexible, ensure that components using this composable implement their own ownership validation where necessary.
🏁 Script executed:
#!/bin/bash # Search for components using this composable to verify ownership checks ast-grep --pattern 'import { useTradeActionClick } from "@/composables/useTradeActionClick" $$$ const { onTradeActionClick } = useTradeActionClick($$$) $$$'Length of output: 158
Manual Verification Required: Review Impact of Parameter Change on Owner Checks
The new composable now accepts an optional
disabledparameter instead of an explicitisOwnercheck. This change improves flexibility but shifts the responsibility of ownership validation to the components consuming the composable. Since the initial search usingast-grepdid not yield any output, please manually verify that:
- All components using
useTradeActionClickare correctly implementing their own ownership or validation logic where required.- The removal of the owner-check logic does not inadvertently allow trade actions in contexts where additional validation is necessary.
components/swap/CreateCollectionSwapButton.vue (1)
31-37: Avoid non-null assertions in collection ID.The use of non-null assertions (
!) oncollectionId.valuemight be unsafe.components/trade/makeOffer/MakeOfferModal.vue (1)
29-32: LGTM! Clean prop addition for conditional price display.The addition of the
:show-priceprop with a Boolean value derived from the first offer item's price existence is a clean implementation that follows Vue.js best practices.queries/subsquid/general/highestOfferByCollectionId.graphql (1)
1-8: LGTM! Well-structured query for fetching highest collection offer.The query is optimized and includes all necessary fields. The descending price order with limit ensures efficient retrieval of the highest offer.
queries/subsquid/general/collectionByIdMinimalWithRoyalty.graphql (1)
22-28: LGTM! Efficient floor price implementation.The
floorPricefield is well-implemented with appropriate filters for non-burned NFTs and non-zero prices, ensuring accurate floor price calculation.i18n/locales/en.json (1)
1524-1525: LGTM! Clear and consistent localization strings for collection swap feature.The new strings are well-organized and maintain consistency with the existing localization structure. The messages are clear and user-friendly, effectively communicating the collection swap functionality.
Also applies to: 1528-1529, 1843-1844, 1847-1849, 1853-1854
|
@Jarsen136 a test is failing , unrelated ? |
I rerun the test and it has passed. |
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
i18n/locales/en.json (1)
1524-1528: Ensure Consistency for Collection Offer Keys in the "offer" Block
The keys"anyNftFromCollection"and"createCollectionOffer"have been added to support collection offers. Please verify that the wording clearly distinguishes these actions from similar keys in the swap workflow. If these keys are intended to be used in different UI contexts, consider adding more context (e.g., using dynamic placeholders where needed) or confirming that the current texts align with the design guidelines.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
i18n/locales/en.json(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: Redirect rules
- GitHub Check: Header rules
- GitHub Check: Pages changed
- GitHub Check: Redirect rules
- GitHub Check: Header rules
- GitHub Check: Pages changed
- GitHub Check: shard (10, 10)
- GitHub Check: shard (9, 10)
- GitHub Check: shard (8, 10)
- GitHub Check: shard (7, 10)
- GitHub Check: shard (6, 10)
- GitHub Check: shard (5, 10)
- GitHub Check: shard (4, 10)
- GitHub Check: shard (3, 10)
- GitHub Check: build-cf
- GitHub Check: shard (2, 10)
- GitHub Check: runner / eslint
- GitHub Check: shard (1, 10)
- GitHub Check: build
- GitHub Check: Cloudflare Pages: koda-art-prod
🔇 Additional comments (4)
i18n/locales/en.json (4)
1843-1843: Dynamic Parameter in "anyNftFromCollection" for Swaps
The new key"anyNftFromCollection": "Any NFT from \"{0}\" Collection"in the "swap" block now includes a parameter placeholder to dynamically inject the collection name. This enhances contextual clarity in the swap interface. Please confirm that the UI correctly passes the{0}parameter and that the placeholder syntax matches the localization framework’s requirements.
1847-1847: Clear Swap Description for Collection-Based Offers
The key"collectionSwapDescription": "Any NFT from this collection will be offered in the swap"clearly informs users of the swap’s nature in a collection context. This addition aligns with the new feature. Ensure it is consistently reflected in any related UI tooltips or documentation.
1848-1848: Concise Preview Label for Collection Swaps
The key"collectionSwapPreview": "Collection Swap"provides a brief label for preview elements in collection swaps. This concise copy is effective; however, double-check that it harmonizes with similar labels in other parts of the application (e.g., in trade or offer modals) for a uniform user experience.
1853-1853: Action Label for Creating a Collection Swap
The new key"createCollectionSwap": "Create Collection Swap"clearly designates the action to initiate a swap using an entire collection. It’s important to ensure that the button and its associated actions in the UI reference this key correctly.
Thank you for your contribution to the Koda - Generative Art Marketplace.
👇 __ Let's make a quick check before the contribution.
PR Type
Needs Design check
Needs QA check
Context
The next PR will be supporting the create collection offer.
Screenshot 📸
Summary by CodeRabbit
New Features
CreateCollectionSwapButtonandCreateCollectionOfferButtonfor initiating collection swaps and offers.TradeActivityTablebased on trade type.highestOfferByCollectionIdGraphQL query to retrieve offers for collections.CollectionPreviewcomponent for enhanced collection display.Improvements