Combo Markets & Token Update #2522
Conversation
…rm to include combo tokens integration, add check for legacy markets sell/buy
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 22
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
components/create/editor/ResetButton.tsx (1)
56-58: Modal does not close after clearing the form.After clicking "Clear", the modal remains open because
setShowResetConfirmation(false)is not called. The user must then click "Cancel" or outside the modal to dismiss it, which creates a confusing experience.Apply this diff to close the modal after reset:
onClick={() => { editor.reset(); + setShowResetConfirmation(false); }}components/create/editor/inputs/FeeSelect.tsx (1)
85-96: Validate numeric input to prevent NaN state.
parseFloatreturnsNaNfor empty strings or invalid input (e.g., when the user clears the field or types non-numeric text). ThisNaNpropagates into the component state unchecked. While line 139's coercion masks it as "0" in the display, the underlying state remains invalid, which may cause downstream validation or calculation issues.Add a guard to handle invalid input explicitly:
const handleCustomInputChange: ChangeEventHandler<HTMLInputElement> = ( event, ) => { const fee = parseFloat(event.target.value); + // Allow NaN to represent "empty" or invalid state, or validate: + if (event.target.value !== "" && Number.isNaN(fee)) { + return; // Ignore invalid non-numeric input + } onChange({ type: "change", target: { name, - value: { value: fee, type: "custom" }, + value: { + value: event.target.value === "" ? undefined : fee, + type: "custom" + }, }, }); };Note: This requires adjusting the
Feetype to allowvalue: number | undefined.components/markets/MarketSearch.tsx (1)
85-103: Increase input padding so text no longer overlaps the icon.With the icon positioned at
left-3(12 px) and sized at 18 px, it occupies horizontal space up to ~30 px. The input still usespl-4(16 px), so the first characters render on top of the icon, making the search term hard to read. Please bump the left padding to clear the icon.- className={`h-11 w-full touch-manipulation overflow-hidden text-ellipsis rounded-lg bg-white/10 pl-4 pr-4 text-base text-white outline-none ring-2 ring-transparent backdrop-blur-sm transition-all placeholder:text-white/60 focus:bg-white/15 focus:shadow-lg focus:ring-2 focus:ring-ztg-green-500/30 md:text-base`} + className={`h-11 w-full touch-manipulation overflow-hidden text-ellipsis rounded-lg bg-white/10 pl-11 pr-4 text-base text-white outline-none ring-2 ring-transparent backdrop-blur-sm transition-all placeholder:text-white/60 focus:bg-white/15 focus:shadow-lg focus:ring-2 focus:ring-ztg-green-500/30 md:text-base`}components/markets/market-filter/index.tsx (1)
91-145: Restore local state updates so callbacks fire again
add,remove,updateOrdering,updateLiquidityOnly, andupdateMarketTypenow only touchqueryState.updateQuery. Because the corresponding setters (setActiveFilters,setActiveOrdering,setWithLiquidityOnly,setMarketType) are never called, theuseEffecthooks that invokeonFiltersChange,onOrderingChange,onWithLiquidityOnlyChange, andonMarketTypeChangestop firing after the first render. Downstream consumers will never hear about user changes, so the list stops reacting. Please set the local state before updating the query, e.g.:const add = (filter: MarketFilter) => { const currentFilters = getFiltersFromQueryState(queryState); if (findFilterIndex(currentFilters, filter) !== -1) return; const nextFilters = [...currentFilters, filter]; + setActiveFilters(nextFilters); queryState.updateQuery({ filters: convertFiltersToQueryFormat(nextFilters), }); }; const remove = (filter: MarketFilter) => { const currentFilters = getFiltersFromQueryState(queryState); const idx = findFilterIndex(currentFilters, filter); if (idx === -1) return; const nextFilters = [ ...currentFilters.slice(0, idx), ...currentFilters.slice(idx + 1), ]; + setActiveFilters(nextFilters); queryState.updateQuery({ filters: convertFiltersToQueryFormat(nextFilters), }); }; const updateOrdering = (ordering: MarketsOrderBy) => { + setActiveOrdering(ordering); queryState.updateQuery({ ordering }); }; const updateLiquidityOnly = (liquidityOnly: boolean) => { + setWithLiquidityOnly(liquidityOnly); queryState.updateQuery({ liquidityOnly }); }; const updateMarketType = (marketType: MarketType) => { + setMarketType(marketType); queryState.updateQuery({ marketType }); };components/account/OnboardingModal.tsx (1)
171-201: Add a key to the fragment wrapper.React will still warn here because the fragment returned from
maplacks a key (the key on the button doesn’t help). Wrapping in<Fragment key={index}>…</Fragment>resolves the warning cleanly.- return ( - <> - <button - key={index} + return ( + <Fragment key={index}> + <button disabled={button.disabled || !isDisclaimerChecked} onClick={button.onClick} className={`flex min-h-[56px] w-full items-center justify-center rounded-lg bg-mystic p-2 text-center hover:bg-gray-100 ${button?.className} ${ button.disabled === true || !isDisclaimerChecked ? "cursor-not-allowed bg-gray-light-2 opacity-50" : "border" }`} > ... - </div> - </> + </div> + </Fragment>And update the import to pull
Fragment:-import { Dispatch, SetStateAction, useEffect, useState } from "react"; +import { Dispatch, Fragment, SetStateAction, useEffect, useState } from "react";components/create/editor/Editor.tsx (1)
171-209: One Page mode hides most of the form
isCurrentstill depends solely oncurrentStep.label, but the newisWizardtoggle defaults tofalse, which hides the stepper. In that default “One Page” mode the user never advancescurrentStep, so only the “Question & Answers” block renders; currency, timelines, moderation, etc. stay hidden and the form can’t be completed unless the user discovers and re-enables the wizard. Please gate visibility onisWizard(e.g. render all sections whenisWizardis false) or otherwise provide navigation so the non-wizard flow remains usable.
♻️ Duplicate comments (17)
components/portfolio/CreatedMarketsTabGroup.tsx (1)
45-49: Clamp the tab index before passing it toTab.Group.
createdMarketsSubTabs.indexOf(createdMarketsTabSelection)returns-1when the query param is missing or unexpected. That-1flows straight intoselectedIndex, which causes Headless UI’sTab.Groupto misbehave (no tab selected / runtime warnings). Compute a safe index first and fall back to0so the component always provides a valid non-negative index.const { regularMarketPools, multiMarketPools } = useMemo(() => { if (!pools) { return { regularMarketPools: null, multiMarketPools: null }; } @@ - return ( + const selectedTabIndex = useMemo(() => { + if (!createdMarketsTabSelection) { + return 0; + } + const idx = createdMarketsSubTabs.indexOf(createdMarketsTabSelection); + return idx >= 0 ? idx : 0; + }, [createdMarketsTabSelection]); + + return ( <Tab.Group defaultIndex={0} - selectedIndex={ - createdMarketsTabSelection && - createdMarketsSubTabs.indexOf(createdMarketsTabSelection) - } + selectedIndex={selectedTabIndex} onChange={(index) => setCreatedMarketsTabSelection(createdMarketsSubTabs[index]) } >components/create/editor/inputs/FeeSelect.tsx (1)
139-139: Display an empty field instead of "0" when the custom fee is unset.This line still shows "0" when
value?.valueisundefinedorNaN, similar to the concern raised in the past review. When a user clears the field or hasn't entered a value yet, displaying "0" is misleading—an empty string is more appropriate.Apply this fix (same as suggested in the previous review):
- value={Number(value?.value || 0).toString()} + value={ + value?.value !== undefined && !Number.isNaN(value.value) + ? value.value.toString() + : "" + }This ensures the input remains blank until the user provides a valid number, improving clarity and matching typical form behavior.
components/liquidity/LiquidityModalAmm2.tsx (1)
43-47: Pool query regresses for legacy markets
PassingpoolId ?? nulldisablesuseAmm2PoolwheneverpoolIdis absent, so the query never runs for legacy (marketId-only) pools and the modal stays empty. Restore the previous fallback by supplying the marketId as the second argument when no explicit poolId is known. This keeps the query enabled and preserves legacy liquidity flows.const { data: pool } = useAmm2Pool( poolId ? 0 : marketId, - poolId ?? null, + poolId ?? marketId, virtualMarket, );components/liquidity/MarketLiquiditySection.tsx (1)
104-113: Guard Decimal against undefined liquidity
stats?.[0]?.liquidityandneoPoolLiquidityare both undefined until data arrives, sonew Decimal(undefined)still throws and crashes the section during the initial render. Provide a numeric fallback (e.g., 0) before constructing the Decimal.- const liquidity = new Decimal(stats?.[0]?.liquidity ?? neoPoolLiquidity); + const rawLiquidity = stats?.[0]?.liquidity ?? neoPoolLiquidity ?? 0; + const liquidity = new Decimal(rawLiquidity);components/liquidity/PoolSettings.tsx (1)
224-235: Fix the NaN display issue in the Base Liquidity input.The input value is set to
parseFloat(baseAssetAmount)which renders "NaN" whenbaseAssetAmountis empty. This prevents users from having a blank field.Apply this diff to fix:
- value={`${parseFloat(baseAssetAmount)}`} + value={baseAssetAmount}Then update the onChange handler to pass the raw string instead of coercing to number, and only use
parseFloatwhere a numeric value is actually needed for calculations.components/markets/MarketOutcomeDisplay.tsx (1)
80-87: Render zero outcomes instead of the skeletonLine 81 currently gates rendering on
outcome ? …, so a legitimate outcome of0(common for scalar/yes-no markets) is treated as falsy and the skeleton shows forever. Please switch to an explicit nullish check so zero renders correctly.- {outcome ? ( + {outcome !== null && outcome !== undefined ? (components/outcomes/CategoricalReportBox.tsx (1)
38-38: Critical: Rules of Hooks violation (duplicate issue)The early return on line 38 precedes hook calls on lines 50-52 and 72, violating React's Rules of Hooks and causing potential runtime errors (including the reported error when loading combo pools in another tab).
Please apply the comprehensive fix already provided in the past review comments, which removes the early return, updates state initialization to allow undefined, adds a useEffect to set the initial selection, and guards the render with conditional checks.
Also applies to: 50-52, 72-72
components/create/editor/inputs/answers/Categorical.tsx (1)
237-243: Add accessible label for remove button (duplicate issue).The remove button lacks an
aria-label, making it inaccessible to screen reader users.Please apply the fix suggested in the past review comments:
<button type="button" className="rounded-md border-2 border-ztg-red-500/60 bg-ztg-red-500/20 px-1.5 py-0.5 text-xs font-medium text-ztg-red-400 transition-all hover:bg-ztg-red-500/30 active:scale-95" onClick={onClear} + aria-label={`Remove ${value || 'answer'}`} > ✕ </button>components/create/editor/Summary.tsx (2)
132-139: Still dereferencing undefined when USD price is loading.
useAssetUsdPriceyieldsundefineduntil the quote arrives, sobaseAssetPrice?.mul(...).toFixed(2)still evaluates.toFixedonundefinedand crashes the summary—exactly the bug flagged earlier. Guard both operands and show a fallback until the price is ready.- <span className="font-medium text-ztg-primary-200"> - ${baseAssetPrice?.mul(baseAmount || 0).toFixed(2)} - </span> + {baseAssetPrice && baseAmount ? ( + <span className="font-medium text-ztg-primary-200"> + $ + {baseAssetPrice + .mul(new Decimal(baseAmount)) + .toFixed(2)} + </span> + ) : ( + <span className="text-ztg-primary-200">--</span> + )}
303-324: Prevent.toFixedon missing liquidity quotes.
When a liquidity row lacks a nestedprice(or the USD price hasn’t loaded),answerLiquidity?.price.pricethrows andbaseAssetPrice?.mul(...).toFixed(2)hits the same undefined dereference. Gate both values before formatting and fall back to--just like we discussed previously.- {answerLiquidity ? ( - <> - {new Decimal(answerLiquidity?.amount || 0) - .mul(answerLiquidity?.price.price ?? 0) - .toFixed(1)}{" "} - <span className="text-ztg-primary-400">≈</span>{" "} - {baseAssetPrice - ?.mul(answerLiquidity?.amount || 0) - .mul(answerLiquidity?.price.price ?? 0) - .toFixed(2)}{" "} - USD - </> - ) : ( - "" - )} + {answerLiquidity?.price?.price != null ? ( + <> + {new Decimal(answerLiquidity.amount ?? 0) + .mul(answerLiquidity.price.price) + .toFixed(1)}{" "} + <span className="text-ztg-primary-400">≈</span>{" "} + {baseAssetPrice + ? baseAssetPrice + .mul(new Decimal(answerLiquidity.amount ?? 0)) + .mul(answerLiquidity.price.price) + .toFixed(2) + : "--"}{" "} + USD + </> + ) : ( + "--" + )}components/create/editor/ComboEditor.tsx (1)
771-793: Spot prices shouldn’t look like USD and min bound should match validation.
We’re showing probabilities, not dollar amounts, and the validator requires ≥ 0.5%. Please drop the$prefix and align the input’sminwith the server-side constraint.- <div className="text-xl font-bold text-white"> - ${spotPrice} - </div> + <div className="text-xl font-bold text-white"> + {spotPrice} + </div> ... - min="0" + min="0.5"components/markets/market-card/MarketOrComboCard.tsx (4)
24-26: Dynamic import still drops the named export.
Same problem as before:dynamic()expects a default, butMarketFavoriteToggleis named. This resolves toundefinedand Next.js throws once the component mounts. Select the named export explicitly.-const MarketFavoriteToggle = dynamic(() => import("../MarketFavoriteToggle"), { - ssr: false, -}); +const MarketFavoriteToggle = dynamic( + () => import("../MarketFavoriteToggle").then((m) => m.MarketFavoriteToggle), + { ssr: false }, +);
338-339: Stop forcing spot prices from block 0.
Passing0forces the query to look at the genesis block, so every combo card reports zero liquidity/flat prices. Let the hook fetch the latest block.- const { data: spotPrices } = useMarketSpotPrices(poolId, 0, virtualMarket); + const { data: spotPrices } = useMarketSpotPrices( + poolId, + undefined, + virtualMarket, + );
358-379: Map “index” is the asset key, so the leading outcome label is wrong.
Map.forEachsupplies(value, key), not a numeric index. Using that key to indexcategorieseither picks the wrong label or falls back toCombination [object Object]. Track a manual counter (or iterate the virtual market ordering) to keep labels lined up.- let highestPrice = new Decimal(0); - let highestIndex = 0; - let highestPercentage = 0; - - spotPrices.forEach((price, index) => { - if (price.gt(highestPrice)) { - highestPrice = price; - highestIndex = index; - } - }); + let highestPrice = new Decimal(0); + let highestIndex = 0; + let highestPercentage = 0; + let currentIndex = 0; + + spotPrices.forEach((price) => { + if (price.gt(highestPrice)) { + highestPrice = price; + highestIndex = currentIndex; + } + currentIndex += 1; + });
287-304: Guardstats.volumebefore constructing a Decimal.
new Decimal(undefined)throws, so combo cards without volume data still crash. Mirror the skeleton fallback you already use for participants/liquidity.- <div className="flex items-center gap-1"> - <BarChart2 size={12} className="text-white/90" /> - <span> - {formatNumberCompact( - new Decimal(stats.volume).div(ZTG).toNumber(), - 2, - )} - </span> - </div> + {stats.volume != undefined ? ( + <div className="flex items-center gap-1"> + <BarChart2 size={12} className="text-white/90" /> + <span> + {formatNumberCompact( + new Decimal(stats.volume).div(ZTG).toNumber(), + 2, + )} + </span> + </div> + ) : ( + <Skeleton width={30} height={12} /> + )}components/create/editor/Publishing.tsx (1)
174-183: Hard-coded event index will break on the next runtime upgrade.
event.event.index === "0x3903"depends on today’s metadata layout; once pallet order shifts, you’ll stop finding the event and treat successful market creation as a failure—the same concern already raised earlier. Match by section/method (or use the generated type guard) instead.- const marketCreationEvent = result.raw.events.find( - (event) => event.event.index.toString() === "0x3903", - ); + const marketCreationEvent = result.raw.events.find( + (event) => + event.event.section === "markets" && + event.event.method === "MarketCreated", + );components/markets/MarketsList.tsx (1)
77-77: Key stability issue persists from previous review.The key construction still switches between
marketIdandpoolIdbased on type, which can cause React to reuse components incorrectly if IDs overlap. The past review recommended usingitem.slugfor a stable, unique identifier.Apply this diff:
<MarketOrComboCard - key={`${item.type}-${item.type === "market" ? item.data.marketId : item.data.poolId}`} + key={item.slug} item={item} />
🧹 Nitpick comments (25)
components/icons/ZtgIcon.tsx (2)
1-13: Consider the default fillColor value.The default
fillColor = "white"may cause visibility issues on light backgrounds. Consider either making this prop required, using a more neutral default (e.g., "currentColor" to inherit text color), or ensuring the icon is only used in contexts where white is appropriate.
14-29: Consider adding accessibility attributes.The SVG lacks accessibility attributes. Depending on usage context:
- If decorative: add
aria-hidden="true"- If semantic: add a
titleprop and<title>element, oraria-labelThis improves the experience for screen reader users.
components/create/editor/ResetButton.tsx (1)
44-46: Consider adding explicit border width for consistency.The Cancel button specifies
border-gray-300without an explicit width (border-1orborder-2), while the Clear button usesborder-2 border-gray-300. For consistent styling and to ensure the border is visible, consider adding an explicit border width to the Cancel button.Apply this diff to add explicit border width:
-className="rounded-full border-gray-300 px-6 py-3 text-sm transition-all duration-200 ease-in-out active:scale-95" +className="rounded-full border-2 border-gray-300 px-6 py-3 text-sm transition-all duration-200 ease-in-out active:scale-95"components/markets/MarketAddresses.tsx (1)
165-175: LGTM: Judgement color standardized.The update from
text-vermiliontotext-ztg-red-400for error states aligns with the design token system for consistent error/warning coloring.Note: Codebase scan found remaining instances of old color classes that could be migrated for consistency:
text-vermilionin pages/deposit.tsx:264text-sky-600in archived/leaderboard/[period].tsx:580 and styles/date-picker.css:6Consider addressing these in a follow-up to complete the migration.
components/account/WalletIcon.tsx (2)
24-39: Consolidate styling to use Tailwind classes consistently.The button mixes Tailwind utilities with redundant inline styles:
px-3(Tailwind) duplicatespadding: "0 12px"(inline)- Height constraints could use
h-11instead of three inline declarationslineHeight: "44px"may be unnecessary for a flex containerboxSizing: "border-box"is typically the defaultThis mixing hurts maintainability and can cause specificity conflicts.
Consider this refactor:
<button key={extensionName} - className={`flex w-full cursor-pointer items-center justify-center rounded-lg border-2 px-3 shadow-sm backdrop-blur-sm transition-all ${ + className={`flex h-11 w-full cursor-pointer items-center justify-center rounded-lg border-2 px-3 shadow-sm backdrop-blur-sm transition-all ${ hasError ? "border-ztg-red-500/40 bg-ztg-red-900/30 hover:border-ztg-red-500/60 hover:bg-ztg-red-900/50" : "border-white/10 bg-white/10 hover:border-white/20 hover:bg-white/20 hover:shadow-md" }`} - style={{ - height: "44px", - minHeight: "44px", - maxHeight: "44px", - lineHeight: "44px", - padding: "0 12px", - boxSizing: "border-box", - }} onClick={onClick} >
41-58: Remove redundant centering container.The nested div (lines 41-49) duplicates the parent button's centering behavior, which already uses
items-center justify-center. The Image's inline styles (line 56) also duplicate its width/height props.Simplify to:
onClick={onClick} > - <div - style={{ - height: "28px", - width: "28px", - display: "flex", - alignItems: "center", - justifyContent: "center", - }} - > - <Image - className={className || ""} - width={28} - height={28} - alt={logoAlt} - src={logoSrc} - style={{ height: "28px", width: "28px" }} - /> - </div> + <Image + className={className} + width={28} + height={28} + alt={logoAlt} + src={logoSrc} + /> </button>components/markets/market-filter/DropDownSelect.tsx (3)
22-31: Consider deriving filter type from a more maintainable source.The hardcoded
labelToTypeMapcreates tight coupling between component labels and filter types. If labels change elsewhere in the codebase, this mapping will silently break. Additionally, the fallback logic withtoLowerCase()adds unnecessary complexity.Consider either:
- Passing the filter type explicitly as a prop to avoid deriving it from the label
- Using a shared constant/enum for label-to-type mappings
- Having the parent component handle this mapping
Apply this diff to pass filter type explicitly:
const DropDownSelect = ({ label, + filterType, options, onChange, portal, isOpen = false, }: { label: string; + filterType: string; options: MarketFilter[]; portal?: HTMLDivElement; isOpen?: boolean; onChange: (val: MarketFilter) => void; }) => {Then simplify the Control component:
- // Map label to filter type - const labelToTypeMap: Record<string, string> = { - Category: "tag", - Currency: "currency", - Status: "status", - }; - - const filterType = - labelToTypeMap[String(selectProps.placeholder || "")] || - (typeof selectProps.placeholder === "string" ? selectProps.placeholder.toLowerCase() : ""); + const filterType = selectProps.filterType;
126-151: Simplify redundant hiding styles.Both
InputandValueContainercomponents apply hiding styles twice: once via Tailwind classes and again via inline styles. This redundancy is unnecessary and could confuse future maintainers.Apply this diff to use only Tailwind classes:
const Input = (props: InputProps<MarketFilter, false>) => { return ( <components.Input {...props} className="!pointer-events-none !absolute !m-0 !h-0 !w-0 !p-0 !opacity-0" - style={{ - position: "absolute", - opacity: 0, - pointerEvents: "none", - width: 0, - height: 0, - margin: 0, - padding: 0, - }} /> ); };The same pattern appears in the
Controlcomponent's children wrapper (lines 61-63) and could be simplified there as well.
153-155: Add proper type annotation for SingleValue.The
SingleValuecomponent usesanyfor its props parameter, which bypasses TypeScript's type checking.Apply this diff to add proper typing:
+import { SingleValueProps } from "react-select"; + -const SingleValue = (props) => { +const SingleValue = (props: SingleValueProps<MarketFilter, false>) => { return <></>; };components/create/editor/inputs/FeeSelect.tsx (2)
26-83: Reconsider the blur-on-change pattern and remove type assertions.Two concerns with the select handler:
Blur timing: Firing
onBlurimmediately afteronChange(lines 41-47, 57-63, 74-80) doesn't match typical form semantics. Blur events should fire when the user navigates away from the field, not on every selection. This may trigger premature validation or unexpected side effects.Type safety: The
as anycasts at lines 54 and 61 bypass TypeScript's type checking when settingundefined. Instead, adjustFeeInputProps.valueto bevalue?: Fee | undefinedif needed, or use a discriminated union to model the "no selection" state explicitly.Consider this approach for the blur events:
- onBlur?.({ - type: "blur", - target: { - name, - value: newValue, - }, - });And remove the
as anycasts by properly typing the undefined case in your FormEvent or value prop.
33-33: Consider starting with an empty custom fee field.Initializing the custom fee to
0means users see "0" in the input field (line 139), which they must clear before entering their value. Starting with an empty field (e.g.,value: undefinedorNaN) would provide a cleaner UX by prompting immediate input without requiring deletion.components/portfolio/Breakdown.tsx (1)
137-143: Consider documenting the change percentage threshold.The color logic treats percentage changes below 1% (
< 0.01) as neutral (white) rather than green. This threshold behavior differs from typical implementations where any positive change appears green. If this is intentional to reduce visual noise from small fluctuations, consider adding a comment explaining the rationale.components/account/WalletSelect.tsx (2)
137-164: Consider accessibility improvements for the numbered list.The decorative numbered circles (lines 139-141, 145-147, etc.) alongside the semantic
<ol>list might create confusion for screen reader users, as they'll hear numbers announced twice. Consider addingaria-hidden="true"to the decorative number spans.Apply this diff to improve accessibility:
<li className="flex items-start gap-2"> - <span className="mt-0.5 flex h-5 w-5 shrink-0 items-center justify-center rounded-full bg-white/10 text-[10px] font-bold text-white/70"> + <span aria-hidden="true" className="mt-0.5 flex h-5 w-5 shrink-0 items-center justify-center rounded-full bg-white/10 text-[10px] font-bold text-white/70"> 1 </span> <span>Open Nova Wallet app on your mobile device</span> </li>Apply similar changes to the other numbered list items (lines 145-147, 151-153, 157-159).
194-223: Consider adding ARIA live region for error announcements.The error display provides good visual feedback, but it should also announce errors to screen reader users. Consider adding
role="alert"oraria-live="polite"to ensure errors are announced when they appear.Apply this diff to improve accessibility:
{/* Error Display */} {errors.length > 0 && ( - <div className="mt-4 space-y-2"> + <div className="mt-4 space-y-2" role="alert" aria-live="polite"> {errors.map((error, index) => ( <div key={index} className="flex items-start gap-2.5 rounded-lg border border-ztg-red-500/40 bg-ztg-red-900/20 p-3 backdrop-blur-sm" >components/markets/MarketContextActionOutcomeSelector.tsx (3)
61-88: Consider extracting asset resolution logic into a separate function.The nested branching logic for determining
assetIndex,category, andcoloris complex and harder to test in its current form. Consider extracting this into a dedicated utility function.Example refactor:
type ResolvedAsset = { assetIndex: number; category: { name: string } | null; color: string; }; const resolveAssetMetadata = ( asset: MarketOutcomeAssetId | CombinatorialToken, index: number, defaultColor: string, market?: FullMarketFragment, outcomeCombinations?: { assetId: CombinatorialToken; name: string; color?: string }[] ): ResolvedAsset => { // Extract the logic from lines 61-88 here // This makes it easier to test and reason about };Then use it in the mapping:
return options.map((asset, index) => { const resolved = resolveAssetMetadata( asset, index, colors[index], market, outcomeCombinations ); return { asset, ...resolved, }; });
134-146: Consider a more informative fallback for missing category names.The function returns an empty string when a category name cannot be resolved (line 139). This could result in a blank selection display, which might confuse users.
Consider returning a more informative fallback:
const getSelectedText = () => { if (isCombinatorialToken(selected)) { return ( assetOptions.find( (a) => JSON.stringify(a.asset) === JSON.stringify(selected), - )?.category?.name ?? "" + )?.category?.name ?? "Unnamed Outcome" ); } else if ("CategoricalOutcome" in selected) { - return market?.categories?.[selected.CategoricalOutcome[1]]?.name ?? ""; + return market?.categories?.[selected.CategoricalOutcome[1]]?.name ?? "Unknown Category"; } else { return selected.ScalarOutcome[1]; } };
258-258: Remove redundantNumber()conversion.The expression
Number(options?.length)is redundant sinceoptions?.lengthis already a number (or undefined). The comparison works correctly without the wrapper.- {Number(options?.length) > SEARCH_ITEMS_THRESHOLD && ( + {(options?.length ?? 0) > SEARCH_ITEMS_THRESHOLD && (Apply the same change on line 274.
Also applies to: 274-274
components/create/editor/ErrorMessage.tsx (1)
41-50: Consider adding accessibility attributes.The error block would be more accessible to screen reader users with ARIA attributes.
- <div className="mt-1.5 flex items-start gap-2 rounded-md border border-ztg-red-500/40 bg-ztg-red-900/20 px-3 py-2 backdrop-blur-sm"> + <div + role="alert" + aria-live="polite" + className="mt-1.5 flex items-start gap-2 rounded-md border border-ztg-red-500/40 bg-ztg-red-900/20 px-3 py-2 backdrop-blur-sm" + > <AlertCircle + aria-hidden="true" className="mt-0.5 shrink-0 text-ztg-red-400" size={16} strokeWidth={2} />components/markets/MarketSearch.tsx (1)
157-165: Fix invalid Tailwind width/ellipsis utilities on the question text.
w-85%andoverflow-ellipsisaren’t valid Tailwind classes, so the element falls back to auto width and never gets the intended truncation behavior. Swap them for supported utilities (e.g.,flex-1 truncate) so the title consistently ellipsizes beside the status badge.- <div className="line-clamp-1 w-85% overflow-ellipsis"> + <div className="line-clamp-1 flex-1 truncate">components/confirmation/ConfirmationProvider.tsx (1)
18-29: Explicitly mark the controls as non-submit buttons.Without a
type, these default tosubmitand can accidentally post a surrounding form if this modal is rendered inside one later. Addingtype="button"keeps the intent explicit.- <button + <button + type="button" className="rounded-md bg-white/10 px-4 py-2 text-sm md:text-base font-medium text-white/90 backdrop-blur-sm transition-all hover:bg-white/20" onClick={() => dismiss(id)} > ... - <button + <button + type="button" className="rounded-md bg-ztg-green-600/80 px-4 py-2 text-sm md:text-base text-white shadow-sm backdrop-blur-sm transition-all hover:bg-ztg-green-600 hover:shadow-md" onClick={() => confirm(id)} >DESIGN_SYSTEM.md (1)
291-297: Consider adding a language identifier to the fenced code block.The z-index hierarchy block could specify a language (e.g.,
text) for consistency with other code blocks in the document.Apply this diff:
-``` +```text z-50: Nested mobile submenus (categories, create market) z-40: TopBar and primary mobile menus z-30: QuickNav (when separate component) z-20: Menu item buttons z-10: Overlays and modals</blockquote></details> <details> <summary>components/front-page/LatestTrades.tsx (1)</summary><blockquote> `17-21`: **Consider proper routing implementation when re-enabling Market column.** The Market column is currently commented out, which sidesteps the linking issue flagged in the previous review (where combo trades would incorrectly link to `/markets/0`). If you plan to re-enable this column, ensure the implementation uses `poolId` for combinatorial pools (linking to `/multi-market/${poolId}`) as suggested in the past review. Also applies to: 86-94 </blockquote></details> <details> <summary>components/markets/MarketChart.tsx (1)</summary><blockquote> `66-147`: **Approve validation improvements; consider optimizing combinatorial sort.** The enhanced validation (lines 66-75, 128-136) properly guards against NaN and invalid prices, addressing the past review concern about missing price handling. The combinatorial market sorting logic (lines 88-125) could be optimized. Currently, `findIndex` is called inside the sort comparator for every comparison, resulting in O(n² × m) complexity where m is the number of outcome assets. Consider pre-computing an index map: ```typescript if (isCombinatorialMarket) { // Pre-compute index map once const assetIndexMap = new Map<string, number>(); market.outcomeAssets.forEach((marketAsset: any, idx: number) => { if (typeof marketAsset === "string") { try { const parsed = JSON.parse(marketAsset); if (parsed.combinatorialToken) { assetIndexMap.set(parsed.combinatorialToken, idx); } } catch {} } else { assetIndexMap.set(JSON.stringify(marketAsset), idx); } }); // Use map for O(1) lookups in sort orderedPrices = [...price.prices].sort((a, b) => { const aIdx = assetIndexMap.get(extractTokenId(a.assetId)) ?? -1; const bIdx = assetIndexMap.get(extractTokenId(b.assetId)) ?? -1; return aIdx - bIdx; }); }This reduces sort complexity from O(n² × m) to O(n log n + m).
components/create/editor/CostCalculator.tsx (2)
138-140: Typo:hasEnoughLiquidtyshould behasEnoughLiquidity.The variable is consistently named throughout the file but misspelled. Consider renaming for correctness.
The typo appears at lines 138, 161, 224, 319, 384, and 716. A global find-and-replace would fix all occurrences:
- const hasEnoughLiquidty = + const hasEnoughLiquidity = ztgBalanceDelta?.gte(0) && (!foreignCurrencyCost || foreignAssetBalanceDelta?.gte(0));
376-387: Improve type safety by replacinganytypes.The parameters
constants,feeDetails, andbaseCurrencyare typed asany, reducing type safety and IntelliSense support.Consider defining proper types or importing existing ones:
import type { ChainConstants } from "lib/hooks/queries/useChainConstants"; import type { FeeAsset } from "lib/hooks/queries/useFeePayingAsset"; import type { CurrencyMetadata } from "lib/constants/supported-currencies"; // Then update the interface: { // ... constants: ChainConstants | null | undefined; // ... feeDetails?: FeeAsset | null; // ... baseCurrency: CurrencyMetadata | null; // ... }Note: Verify the actual exported types from these modules match the suggested names.
| title="Liquidity" | ||
| value={poolZtgTotal?.mul(ZTG) ?? new Decimal(0)} | ||
| usdZtgPrice={props.usdZtgPrice} | ||
| changePercentage={0} |
There was a problem hiding this comment.
Hardcoded change percentage for liquidity.
The changePercentage for the Liquidity slot is hardcoded to 0, which means liquidity position changes won't be tracked or displayed. Consider implementing change tracking for liquidity positions or adding a TODO comment if this is a known limitation.
🤖 Prompt for AI Agents
In components/portfolio/Breakdown.tsx around line 73 the Liquidity slot uses
changePercentage={0} which hardcodes no change; replace this with a real
calculation or a clear TODO: compute changePercentage as ((currentLiquidity -
previousLiquidity) / previousLiquidity) * 100 using the available props/state
(or source a previous value from parent/portfolio data), ensure you handle
divide-by-zero and missing previous values by falling back to 0, or if tracking
changes is intentionally unsupported add a TODO comment explaining the
limitation and a note to wire in previous liquidity data later.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
components/assets/AssetActionButtons/DisputeButton.tsx (1)
34-93: Don’t disable scalar disputes by requiringassetId.
Lines 37-53 now gate the button onassetId, but this prop remains optional because we render<DisputeButton market={market} />for scalar markets. After this change, those buttons are permanently disabled, so scalar markets (and any other flows that intentionally omitassetId) can no longer be disputed. Please only demand an asset index when we actually need one (categorical/combinatorial paths) and keep scalar markets working.Apply something like:
- if (!assetId) return true; - - let assetIndex: number | undefined; - - if (IOMarketOutcomeAssetId.is(assetId)) { + const requiresOutcomeSelection = market.marketType.categorical != null; + + if (requiresOutcomeSelection && !assetId) return true; + + let assetIndex: number | undefined; + + if (assetId && IOMarketOutcomeAssetId.is(assetId)) { assetIndex = getIndexOf(assetId); - } else if (isCombinatorialToken(assetId)) { + } else if (assetId && isCombinatorialToken(assetId)) {…and keep the rest of the logic the same.
components/outcomes/CategoricalDisputeBox.tsx (1)
79-143: Restore real “Previous Bond” value.
bondAmountis now set todisputeBond, so the conditional block that should display prior bond never renders and the number shown would be the new stake, not the actual locked amount. Users lose the context of what is already at risk. Pull the previous bond from the dispute history (or whatever source we used before) instead of mirroringdisputeBond. For example:- const bondAmount = - disputes && isConstantsLoading === false ? disputeBond : undefined; + const previousBond = + disputes && disputes.length > 0 + ? disputes[disputes.length - 1]?.bond + : undefined; + + const bondAmount = + previousBond != null && isConstantsLoading === false ? previousBond : undefined;Adjust the property name if needed, but the key is to use the actual prior stake so the “Previous Bond” panel shows meaningful data again.
♻️ Duplicate comments (2)
components/markets/MarketHistoryModal.tsx (2)
3-3: Replaceanytype with proper OutcomeReport type.This issue was already flagged in a previous review. The
anytype eliminates type safety for outcome handling.
47-53: Fix scalar type check and add validation.This issue was already flagged in a previous review. The logic checks
marketType.scalar === null, but scalar markets have a string array, not null. The check is inverted and the non-null assertion is unsafe.
🧹 Nitpick comments (1)
components/markets/ReportResult.tsx (1)
3-3: Remove unused Twitter-related code.The
TwitterBirdimport and Twitter-related variables (marketUrl,twitterBaseUrl,tweetUrl) are no longer used after removing the Twitter share block from the render output. Clean up this dead code.Apply this diff to remove the unused code:
-import { TwitterBird } from "components/markets/TradeResult"; import { MarketCategoricalOutcome, MarketScalarOutcome, displayOutcome, } from "lib/types";const outcomeName = displayOutcome(market, outcome); - const marketUrl = `https://app.zeitgeist.pm/markets/${market.marketId}`; - - const twitterBaseUrl = "https://twitter.com/intent/tweet?text="; - const tweetUrl = `${twitterBaseUrl}I just reported the outcome of %40ZeitgeistPM market: "${market.question}" to be ${outcomeName}%0A%0ACheck out the market here%3A%0A&url=${marketUrl}`; - return (Also applies to: 22-25
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
components/assets/AssetActionButtons/DisputeButton.tsx(4 hunks)components/markets/DisputeResult.tsx(1 hunks)components/markets/MarketHistoryModal.tsx(1 hunks)components/markets/MarketOutcomeDisplay.tsx(1 hunks)components/markets/ReportResult.tsx(1 hunks)components/outcomes/CategoricalDisputeBox.tsx(5 hunks)components/outcomes/ScalarDisputeBox.tsx(1 hunks)components/outcomes/ScalarReportBox.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- components/markets/MarketOutcomeDisplay.tsx
- components/outcomes/ScalarReportBox.tsx
🧰 Additional context used
🧬 Code graph analysis (3)
components/assets/AssetActionButtons/DisputeButton.tsx (2)
lib/types/combinatorial.ts (1)
isCombinatorialToken(21-24)components/ui/ModalPanel.tsx (2)
ModalPanel(30-67)ModalBody(94-114)
components/markets/MarketHistoryModal.tsx (3)
lib/hooks/queries/useMarketEventHistory.ts (1)
MarketEventHistory(10-16)lib/util/format-scalar-outcome.ts (1)
formatScalarOutcome(5-17)components/markets/MarketHeaderUtils.tsx (1)
UserIdentity(7-25)
components/outcomes/CategoricalDisputeBox.tsx (4)
lib/types/index.ts (1)
CombinatorialToken(71-71)lib/types/combinatorial.ts (1)
isCombinatorialToken(21-24)lib/util/parse-asset-id.ts (1)
parseAssetIdStringWithCombinatorial(15-46)lib/hooks/queries/useMarketDisputes.ts (1)
marketDisputesRootKey(11-11)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (3)
components/markets/ReportResult.tsx (1)
28-28: LGTM! Styling changes align with design system updates.The addition of
text-whiteand removal oftext-ztg-blueare consistent with the PR's goal of standardizing color tokens and reducing reliance on specific accent colors.Also applies to: 30-30
components/markets/DisputeResult.tsx (1)
18-29: Styling refresh looks solid.
The updated spacing and CTA treatment keep the dispute success state consistent with the new modal styling; no functional concerns on this slice.components/outcomes/ScalarDisputeBox.tsx (1)
56-83: UI polish aligns with modal updates.
The reworked bond summary block and full-width confirm button match the refreshed dispute modal styling while preserving existing behaviour.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/markets/ScalarPriceRange.tsx (1)
65-87: Change container background to ensure sufficient contrast for white text.The current styling uses white text (90-100% opacity) on a light translucent background (
bg-white/10), which creates insufficient contrast regardless of what's behind due to the light background color itself. This violates WCAG AA standards (4.5:1 minimum for normal text) and harms readability.Change the container background from
bg-white/10tobg-black/20to provide adequate contrast:- className={`relative flex h-6 w-full items-center overflow-hidden rounded-lg bg-white/10 px-2.5 shadow-md backdrop-blur-sm transition-all sm:h-[30px] sm:px-4 ${className}`} + className={`relative flex h-6 w-full items-center overflow-hidden rounded-lg bg-black/20 px-2.5 shadow-md backdrop-blur-sm transition-all sm:h-[30px] sm:px-4 ${className}`}This maintains the frosted glass aesthetic while ensuring white text meets accessibility standards.
♻️ Duplicate comments (3)
components/liquidity/PoolSettings.tsx (1)
238-246: Keep the Base Liquidity field blank instead of showingNaN.Line 238 still uses
parseFloat(baseAssetAmount)which displays "NaN" when the field is empty or cleared. The input should bind to the raw string value instead.Apply this diff to fix the issue:
- value={`${parseFloat(baseAssetAmount)}`} + value={baseAssetAmount} onChange={(event) => { - const value = parseFloat(event.target.value); - if (!isNaN(value)) { - handleBaseAmountChange(`${value}`); - } else { + const value = event.target.value; + if (value === "" || value === null) { handleBaseAmountChange(""); + } else { + handleBaseAmountChange(value); } }}components/account/AccountButton.tsx (1)
242-251: Fix critical Rules-of-Hooks violation before shipping.The
useEffecton lines 244-246 is being invoked inside the Headless UIMenurender-prop callback, which violates React's "only call hooks at the top level" rule. This will cause inconsistent hook ordering and runtime errors.The previous review comment provides a detailed solution: extract the render-prop body into a child component that receives
open,close, and other props, then run the effect at that component's top level.components/liquidity/MarketLiquiditySection.tsx (1)
103-111: Critical: Decimal instantiation can still receive undefined.The existing review comment correctly identified this issue, but it remains unaddressed. When
stats?.[0]?.liquidityis absent and allneoPoolLiquidityfallbacks fail (totalShares, liquiditySharesManager sum, liquidityParameter all undefined), line 111 will throwDecimalError: Invalid argument: undefined.Additionally, line 104 uses a type assertion
(neoPool as any)?.totalSharesthat bypasses type safety and may access a non-existent property.Apply this diff to guarantee a numeric fallback:
const neoPoolLiquidity = - (neoPool as any)?.totalShares ?? + neoPool?.totalShares ?? neoPool?.liquiditySharesManager?.reduce( (total: any, manager: any) => total + Number(manager.stake), 0, ) ?? neoPool?.liquidityParameter; - const liquidity = new Decimal(stats?.[0]?.liquidity ?? neoPoolLiquidity); + const rawLiquidity = stats?.[0]?.liquidity ?? neoPoolLiquidity ?? 0; + const liquidity = new Decimal(rawLiquidity);
🧹 Nitpick comments (5)
components/markets/SimilarMarketsSection.tsx (1)
48-48: Remove redundant key prop.The parent
divat line 41 already provides the requiredkeyfor the list. Thekeyprop onMarketCardis redundant and can be removed.Apply this diff:
<MarketCard - key={market.marketId} market={market} numParticipants={stat?.participants} liquidity={stat?.liquidity}components/markets/ScalarPriceRange.tsx (1)
42-61: Consider extracting date formatting logic.The date formatting pattern is repeated three times (lines 44-46, 51-53, 58-60). Extracting this into a helper function would improve maintainability.
const formatValue = (value: number, scalarType: ScalarRangeType) => { return scalarType === "date" ? new Intl.DateTimeFormat("default", { dateStyle: "medium" }).format(new Date(value)) : formatNumberCompact(value); }; const lowerDisplay = formatValue(lowerBound, scalarType); const upperDisplay = formatValue(upperBound, scalarType); const positionDisplay = formatValue(position, scalarType);components/create/editor/ComboEditor.tsx (2)
401-408: Consider preserving empty string instead of defaulting to "0".When users clear the input, the sanitization immediately replaces it with "0", which will fail validation (minimum 0.5% for prices, 200 for liquidity). This creates a confusing experience where clearing triggers an instant invalid state.
Consider allowing the field to remain empty until blur or submission, then apply defaults/validation:
// If completely empty, default to "0" - if (sanitizedValue === "") { + if (sanitizedValue === "" || sanitizedValue === ".") { setForm((prev) => ({ ...prev, spotPrices: prev.spotPrices.map((price, i) => - i === index ? "0" : price, + i === index ? "" : price, ), })); return; }Then update validation to handle empty strings by treating them as 0 or showing a clearer "field required" message.
Also applies to: 427-430
499-499: Hardcoded fuel parameter may need configuration.The
{ total: 16, consumeAll: true }fuel settings are fixed. If different market combinations require different computational budgets, this could cause transactions to fail or waste resources.Consider making fuel configurable based on the number of outcomes or exposing it as an advanced option if the default proves insufficient for larger combinations.
components/liquidity/MarketLiquiditySection.tsx (1)
195-198: Track the Buy/Sell Full Sets compatibility work.The TODO indicates that Buy/Sell Full Sets functionality is not yet compatible with the combinatorial token system. This is an important feature for combo markets.
Would you like me to open a new issue to track implementing Buy/Sell Full Sets support for combinatorial tokens?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
public/currencies/usdc.svgis excluded by!**/*.svg
📒 Files selected for processing (6)
components/account/AccountButton.tsx(7 hunks)components/create/editor/ComboEditor.tsx(1 hunks)components/liquidity/MarketLiquiditySection.tsx(5 hunks)components/liquidity/PoolSettings.tsx(8 hunks)components/markets/ScalarPriceRange.tsx(2 hunks)components/markets/SimilarMarketsSection.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
components/create/editor/ComboEditor.tsx (9)
lib/hooks/queries/useMarketSearch.ts (1)
useMarketSearchWithDefaults(58-92)components/ui/Loader.tsx (1)
Loader(14-76)lib/hooks/useSdkv2.ts (1)
useSdkv2(31-67)lib/state/wallet.tsx (1)
useWallet(485-612)lib/state/notifications.ts (1)
useNotifications(122-130)lib/hooks/queries/useAllComboPools.ts (2)
useAllComboPools(12-34)findDuplicateCombo(45-60)lib/hooks/useExtrinsic.ts (1)
useExtrinsic(12-134)lib/util/format-compact.ts (1)
formatNumberCompact(1-12)lib/constants/index.ts (1)
ZTG(12-12)
components/markets/SimilarMarketsSection.tsx (1)
components/markets/market-card/index.tsx (1)
MarketCard(43-195)
components/liquidity/MarketLiquiditySection.tsx (4)
components/ui/Loader.tsx (1)
Loader(14-76)lib/hooks/queries/useMarketsStats.ts (1)
useMarketsStats(18-50)lib/util/format-compact.ts (1)
formatNumberCompact(1-12)lib/constants/index.ts (1)
ZTG(12-12)
components/account/AccountButton.tsx (4)
lib/state/alerts/useAlerts.ts (1)
useAlerts(38-118)lib/hooks/queries/useIdentity.ts (1)
useIdentity(8-73)lib/hooks/useSimpleScrollLock.ts (1)
useSimpleScrollLock(10-34)components/account/NotificationsPanel.tsx (1)
NotificationsPanel(30-176)
components/liquidity/PoolSettings.tsx (1)
components/ui/InfoPopover.tsx (1)
InfoPopover(22-114)
🪛 Biome (2.1.2)
components/account/AccountButton.tsx
[error] 244-244: This hook is being called from a nested function, but all hooks must be called unconditionally from the top-level component.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
🔇 Additional comments (16)
components/markets/SimilarMarketsSection.tsx (1)
26-59: Excellent refactor – the negative z-index issue is resolved.The previous blocking issue with
z-[-1]making MarketCard links unclickable has been fixed. The new card-based layout with conditional rendering, proper stacking context (relativewithout negative z-index), and staggered animations creates a polished, interactive user experience. The header and translucent card styling align well with the design system.components/liquidity/PoolSettings.tsx (2)
78-105: LGTM!The PriceSetter styling updates improve the component's compactness and visual hierarchy. The logic remains sound.
216-272: LGTM!The conditional rendering of the Base Liquidity section via
hideBaseLiquidityprop is well-implemented, and wrapping the Table in anoverflow-x-autocontainer improves responsiveness. The updated column widths and simplified InfoPopover content enhance the UI.components/markets/ScalarPriceRange.tsx (1)
77-77: Verify gradient bar visibility across backgrounds.The gradient bar now uses reduced opacity (
from-ztg-green-500/60 to-ztg-green-400/70), which combined with the translucent container might reduce visibility on certain backgrounds. Since this bar represents the market prediction, ensure it remains clearly visible in all use cases.components/account/AccountButton.tsx (5)
18-44: LGTM - Imports support new features.All new imports are appropriately used for the enhanced account menu functionality including notifications, identity display, mobile positioning, and scroll locking.
58-73: LGTM - Enhanced balance display.The updated BalanceRow component includes improved visual design with glass morphism effects, hover states, and a sensible fallback display for missing balances.
83-83: LGTM - Polished button styling.The HeaderActionButton now includes enhanced visual effects with gradients, blur, and hover shadows while maintaining proper disabled state handling.
154-195: LGTM - Comprehensive dropdown positioning.The positioning logic properly handles:
- Dynamic calculation of dropdown position relative to the button
- Mobile detection with responsive behavior
- Scroll locking on mobile to prevent background scrolling
- Proper cleanup of event listeners
The implementation correctly addresses the challenges of fixed positioning across desktop and mobile viewports.
253-639: LGTM - Comprehensive account menu redesign.The redesigned menu provides:
- Mobile-aware layout with dynamic positioning and close button
- Identity display with proper fallback chain (on-chain identity → wallet name → shortened address)
- Notifications integration with visual indicators and count badge
- Well-organized sections: balances, navigation items, and disconnect
- Consistent glass morphism design with appropriate hover states
- Proper navigation handlers and menu closure on item selection
The implementation successfully handles both desktop and mobile viewports while maintaining a clean, accessible interface.
components/create/editor/ComboEditor.tsx (5)
73-73: 50-result limit balances performance with discoverability.The
.slice(0, 50)cap prevents performance issues but might hide relevant markets if users need to scroll deeper. Given the performance rationale and the presence of search, this is acceptable.
268-290: Clean cartesian-product generation for outcome combinations.The nested loops correctly generate all pairings of market outcomes, assign colors via
calcMarketColors, and build descriptive combination names. The color cycling with modulo ensures robustness even if more combinations exist than colors.
306-362: Comprehensive validation covers all critical constraints.The validation correctly enforces:
- Exactly two markets
- Matching collateral assets
- Duplicate pool detection
- Spot price sum within tolerance (0.01%)
- Individual price bounds (0.5%–95%)
- Minimum liquidity (200 ZTG)
Clear error messages guide users to fix issues before deployment.
630-683: "Assume" / "Then" labeling clarifies conditional logic.The UI distinguishes Market 1 (blue, "Assume") from Market 2 (green, "Then"), making the conditional relationship explicit. The compact card layout with outcome truncation and remove buttons provides good UX for market selection.
696-722: Real-time duplicate warning improves user experience.Showing the duplicate pool warning (with link) before the user attempts deployment is excellent UX. The distinction between an informational warning (orange) and a blocking error (red, set during validation) prevents wasted effort.
components/liquidity/MarketLiquiditySection.tsx (2)
200-200: ****The
marketId={comboMarket ? 0 : neoPool.marketId}pattern is intentional, not a bug. ThegetPoolIdForTransactionutility explicitly checks if a market is combinatorial and returnspool.poolIdfor combinatorial markets instead ofmarketId. WhenmarketId=0for combo markets:
- The component calls
useMarket({ marketId })but has a fallback:activeMarket = virtualMarket || market, ensuring market data is always available via thevirtualMarketprop passed from the parent.- The transaction correctly uses
poolIdForTx(derived fromgetPoolIdForTransaction) rather thanmarketIddirectly in theneoSwaps.joincall.The sentinel value
0is a design pattern for combo markets, not a data integrity issue.Likely an incorrect or invalid review comment.
158-158: ****The
z-10value is correct and follows the established design pattern in the codebase. The design system documentation explicitly designatesz-10for overlays and modals, and this inline tooltip pattern is used consistently throughout the application:MarketHeaderUtils.tsx,ComboMarketHeader.tsx,MarketMetadataBadges.tsx, andScalarPriceRange.tsxall usez-10for similar group-hover tooltips. TheLiquidityHeadercomponent is rendered at the root page level with no higher z-index parent containers that would occlude this tooltip, so no changes are needed.Likely an incorrect or invalid review comment.
| if (poolDeployedEvent) { | ||
| const eventData = poolDeployedEvent.event.data; | ||
|
|
||
| if (!eventData || eventData.length < 3) { | ||
| console.error("Invalid pool deployment event data structure"); | ||
| return; | ||
| } | ||
|
|
||
| const poolId = eventData[2].toString(); | ||
| router.push(`/multi-market/${poolId}`); |
There was a problem hiding this comment.
Fragile event data extraction—add defensive type checks.
The code assumes eventData[2] is a valid pool ID without verifying its type or structure. If the event schema changes or returns unexpected data, .toString() could produce incorrect routing.
Apply this diff to add safer extraction:
if (poolDeployedEvent) {
const eventData = poolDeployedEvent.event.data;
if (!eventData || eventData.length < 3) {
console.error("Invalid pool deployment event data structure");
+ notificationStore.pushNotification(
+ "Pool deployed but could not extract pool ID",
+ { type: "Error" }
+ );
return;
}
- const poolId = eventData[2].toString();
+ const poolIdRaw = eventData[2];
+ if (!poolIdRaw) {
+ console.error("Pool ID not found in event data");
+ return;
+ }
+ const poolId = poolIdRaw.toString();
router.push(`/multi-market/${poolId}`);
}🤖 Prompt for AI Agents
In components/create/editor/ComboEditor.tsx around lines 518 to 527, the code
blindly calls .toString() on eventData[2]; add defensive type checks to ensure
we only derive a poolId when eventData[2] exists and is a valid primitive or
object with a safe toString method. Check that eventData[2] is not
null/undefined, handle common types (string, number, BigNumber-like with a
toString function), coerce to a string via String(value) only after verifying
value !== null, then validate the resulting poolId is non-empty and matches the
expected pattern (e.g., digits or hex) before calling router.push; if validation
fails, log an error with the raw eventData and return without navigating.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
components/assets/AssetActionButtons/ReportButton.tsx (2)
43-56: Combinatorial tokens cannot be reported - disable this flow.Per the previous review feedback, combinatorial pools do not have reports because they combine multiple markets, each of which has its own separate report. The logic here attempts to enable reporting for combinatorial tokens, which is conceptually incorrect.
The report button should either be hidden or disabled when
assetIdis a combinatorial token.Consider applying this approach to explicitly disable reporting for combinatorial tokens:
const { isLoading, isSuccess, send } = useExtrinsic( () => { if (!isRpcSdk(sdk)) return; let outcomeIndex: number | undefined; if (IOCategoricalAssetId.is(assetId)) { outcomeIndex = assetId.CategoricalOutcome[1]; - } else if (isCombinatorialToken(assetId)) { - // For combinatorial tokens, find the index in the market's outcome assets - const tokenHash = assetId.CombinatorialToken; - const index = market.outcomeAssets?.findIndex((outcomeAsset) => - outcomeAsset.includes(tokenHash), - ); - if (index !== undefined && index >= 0) { - outcomeIndex = index; - } } if (outcomeIndex === undefined) return;And update
reportDisabledto explicitly check for combinatorial tokens:const reportDisabled = !isRpcSdk(sdk) || !stage || isLoading || isSuccess || + isCombinatorialToken(assetId) || (stage.type === "OracleReportingPeriod" && !connectedWalletIsOracle);
83-92: Remove combinatorial token handling from outcome name resolution.This logic attempts to resolve the outcome name for combinatorial tokens, which reinforces the incorrect assumption that combinatorial tokens can be reported. As noted in the previous review, only individual markets have reports, not combinatorial pools.
Additionally, the search logic here (lines 85-91) duplicates the logic from lines 47-53. If this were valid logic (which it isn't for reporting), it should be extracted into a helper function.
Remove the combinatorial token branch:
// Get outcome name based on asset type let outcomeName = ""; if (assetId) { if (IOMarketOutcomeAssetId.is(assetId)) { outcomeName = market.categories?.[getIndexOf(assetId)]?.name || ""; - } else if (isCombinatorialToken(assetId)) { - // For combinatorial tokens in multi-market positions, find the outcome by matching the token - const tokenHash = assetId.CombinatorialToken; - const index = market.outcomeAssets?.findIndex((outcomeAsset) => - outcomeAsset.includes(tokenHash), - ); - if (index !== undefined && index >= 0) { - outcomeName = market.categories?.[index]?.name || ""; - } } }components/assets/AssetActionButtons/RedeemButton.tsx (1)
284-289: Combinatorial market detection was previously flagged.This detection logic was flagged in a previous review as potentially unreliable, particularly the check
market.marketType.categorical === "Combinatorial". While the past suggestion was marked as addressed, consider verifying this detection works correctly for all market type combinations.
🧹 Nitpick comments (2)
components/assets/AssetActionButtons/RedeemButton.tsx (2)
105-170: Consider refactoring for maintainability.The value calculation logic spans 65 lines with multiple nested conditions for different market type combinations (partial/full redemption × parent scalar/categorical × child scalar/categorical). This complexity makes the code difficult to understand, test, and maintain.
Consider extracting helper functions such as:
calculatePartialRedemptionValue()calculateScalarCombinationValue()calculateCategoricalValue()Each helper would handle a specific scenario and return the calculated value, improving readability and testability.
321-326: Document the hardcodedtotal: 16parameter.Based on past review discussions, the hardcoded
total: 16is intentional (not derived fromtokenIndex.length). However, without inline documentation, it's unclear why 16 is the correct value for all redemption scenarios.Add a comment explaining the
totalparameter's purpose and why 16 is used:return sdk.api.tx.combinatorialTokens.redeemPosition( parentCollectionId, marketIds, tokenIndex, + // fuel parameter: total=16 represents the maximum computation units, + // consumeAll=true instructs the blockchain to consume all available units { total: 16, consumeAll: true }, );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/assets/AssetActionButtons/RedeemButton.tsx(6 hunks)components/assets/AssetActionButtons/ReportButton.tsx(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
components/assets/AssetActionButtons/ReportButton.tsx (2)
lib/types/combinatorial.ts (1)
isCombinatorialToken(21-24)components/ui/ModalPanel.tsx (2)
ModalPanel(30-67)ModalBody(94-114)
components/assets/AssetActionButtons/RedeemButton.tsx (3)
lib/types/combinatorial.ts (1)
isCombinatorialToken(21-24)lib/constants/index.ts (1)
ZTG(12-12)lib/hooks/useExtrinsic.ts (1)
useExtrinsic(12-134)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (7)
components/assets/AssetActionButtons/ReportButton.tsx (1)
125-141: LGTM! Clean modal UI refactor.The migration to
ModalPanelandModalBodycomponents with an explicit header and close button improves the UX and aligns with the design system updates across the codebase.components/assets/AssetActionButtons/RedeemButton.tsx (6)
26-29: LGTM: Clean imports for combinatorial token support.The import statements correctly bring in the necessary types and utilities for handling combinatorial tokens.
35-38: LGTM: Props appropriately extended for multi-market scenarios.The new optional properties provide the necessary context for handling combinatorial token redemptions across single-market and multi-market cases.
203-224: LGTM: Clean balance display implementation.The conditional rendering for
showBalanceis well-structured, providing a loading state and formatted balance display alongside the redeem button.
334-339: LGTM: Contextual success notification.The notification text appropriately adapts based on whether it's a parent market outcome redemption or a standard base asset redemption.
347-353: LGTM: Enhanced success state UI.The updated success state provides clear visual feedback with color, icon, and text indicating successful redemption.
139-144: The condition is correct and reflects intentional design for scalar market handling.The
market.resolvedOutcome === null && isParentScalarcheck is not incorrect. In this codebase,resolvedOutcome === nullis a sentinel value indicating a scalar parent market, where the blockchain calculates payouts continuously rather than returning a discrete outcome index.This pattern is confirmed by identical logic in
pages/multi-market/[poolid].tsx(lines 522-524 and 1085-1087) with the same comment: "Parent is scalar - show all positions (blockchain calculates payouts)."The condition properly distinguishes between:
- Scalar parent (
resolvedOutcome === null): All positions may have value; blockchain handles calculations- Categorical parent (
resolvedOutcome !== null): Only specific outcome index has valueLines 146 and 164, which safely use
resolvedOutcomewithout null checks, are protected by their context—they only execute for non-scalar scenarios (line 146 checks!isParentScalar, line 164 is in the "Both categorical" section).
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
components/create/editor/inputs/answers/Categorical.tsx (1)
96-116: Fix falsy check that prevents dragging the first item.Line 103 incorrectly uses
!oldIndex || !newIndexto check for missing indices. SincefindIndexreturns0for the first item, this falsy check will incorrectly reject valid drag operations involving the first answer.Apply this diff to fix the logic:
if (over && active.id !== over.id) { const oldIndex = value?.answers.findIndex((v) => v === active.id); const newIndex = value?.answers.findIndex((v) => v === over.id); - if (!oldIndex || !newIndex || !value?.answers) return; + if (oldIndex == null || oldIndex === -1 || newIndex == null || newIndex === -1 || !value?.answers) return; onChange?.({components/create/editor/inputs/FeeSelect.tsx (1)
85-98: Avoid storingNaNin state; useundefinedor a sentinel for empty input.Line 90 assigns
NaNwhen the input is empty, which is then stored in the form state. While the display logic (lines 141-145) filters outNaNbefore rendering, having invalid data in state can cause issues in validation, serialization, or downstream logic. Consider storingundefinedor omitting thevalueproperty when the field is blank.Example refactor:
const handleCustomInputChange: ChangeEventHandler<HTMLInputElement> = ( event, ) => { const inputValue = event.target.value; - // Allow empty string (blank field) or valid numbers - const fee = inputValue === "" ? NaN : parseFloat(inputValue); + const fee = inputValue === "" ? undefined : parseFloat(inputValue); onChange({ type: "change", target: { name, - value: { value: fee, type: "custom" }, + value: fee !== undefined ? { value: fee, type: "custom" } : undefined, }, }); };And adjust the custom input check accordingly:
- {value?.type === "custom" && ( + {value?.type === "custom" && value.value !== undefined && (
♻️ Duplicate comments (6)
components/liquidity/PoolSettings.tsx (1)
238-238: Unresolved: NaN still displays when Base Liquidity field is empty.This is the same issue flagged in the previous review. When
baseAssetAmountis an empty string,parseFloat("")returnsNaN, which is then displayed as the string"NaN"in the input field. Users cannot leave the field blank.Apply the previously suggested fix:
- value={`${parseFloat(baseAssetAmount)}`} + value={baseAssetAmount}components/create/editor/inputs/BlockPeriod.tsx (1)
154-167: Inconsistent visibility: custom date input remains inaccessible.While the custom duration input was fixed to be always accessible (line 140), the custom date input still requires
value?.type === "date"(line 154). This inconsistency means users cannot access the custom date picker from scratch without first selecting another option that produces a date value.Apply this diff to align DateTimePicker visibility with DurationInput:
- {hasCustomDateOption && value?.type === "date" && ( + {hasCustomDateOption && ( <div className="w-full"> <DateTimePicker timezone={timezone} name={name} className={`w-full border-ztg-green-600/80 !bg-ztg-green-600/90 !text-white shadow-md`} placeholder="Custom Date" isValid={value?.type === "date" && isValid} value={chainTime && value?.type === "date" ? value.date : undefined} onChange={handleDateChange} onBlur={handleDateBlur} /> </div> )}components/account/NotificationsPanel.tsx (3)
227-262: Same issues as CourtCaseReadyToSettleItem.This component has identical issues with the non-null assertion on line 234 and overly broad
useEffectdependencies on line 238. Apply the same fixes suggested forCourtCaseReadyToSettleItem.
264-299: Same issues as previous court case alert items.This component has identical issues with the non-null assertion on line 271 and overly broad
useEffectdependencies on line 275. Apply the same fixes suggested forCourtCaseReadyToSettleItem.
301-331: Overly broad useEffect dependencies.Line 310: The dependency array
[alert, router]will cause the effect to re-run when anyalertproperty changes, but onlyalert.market.marketIdis actually used. This can cause unnecessary prefetch calls.Apply this diff:
useEffect(() => { router.prefetch(`/markets/${alert.market.marketId}`); - }, [alert, router]); + }, [alert.market.marketId, router]);components/create/editor/Summary.tsx (1)
284-339: Prevent liquidity summary from crashing when price data is missing
answerLiquidity?.price.pricereintroduces the same crash we flagged earlier: if a liquidity row comes back without a nestedpriceobject, the optional chain stops atanswerLiquidity?.price, and the subsequent.priceaccess throws. We also still callNumber(answerLiquidity?.amount).toFixed(1)unconditionally, which returns"NaN"when the liquidity array is shorter than the answers list. Please guard these lookups before formatting so the summary renders a safe fallback instead of exploding. (developer.mozilla.org){answers?.answers.map((answer, answerIndex) => { const answerLiquidity = liquidity?.rows?.[answerIndex]; + const amount = answerLiquidity?.amount; + const price = answerLiquidity?.price?.price; return ( <div key={answerIndex} className="mb-4 flex-1 rounded-md border-2 border-ztg-primary-200/30 bg-ztg-primary-900/50 px-4 py-3 shadow-sm backdrop-blur-md md:mb-0" > <div className="mb-2 text-base font-semibold uppercase text-ztg-primary-100"> {answerLiquidity?.asset} </div> {answers.type === "categorical" && ( <div className="mb-2 text-sm text-ztg-primary-200">{answer}</div> )} {liquidity && liquidity.deploy && moderation === "Permissionless" ? ( <div className="space-y-1.5 border-t-2 border-ztg-primary-200/30 pt-2.5 text-xs"> <div className="flex justify-between"> <span className="text-ztg-primary-200">Amount:</span> - <span className="font-medium text-ztg-primary-100"> - {Number(answerLiquidity?.amount).toFixed(1) ?? "--"} - </span> + <span className="font-medium text-ztg-primary-100"> + {amount !== undefined ? Number(amount).toFixed(1) : "--"} + </span> </div> <div className="flex justify-between"> <span className="text-ztg-primary-200">Value:</span> <span className="font-medium text-ztg-primary-100"> - {answerLiquidity ? ( + {amount !== undefined && price !== undefined ? ( <> - {new Decimal(answerLiquidity?.amount || 0) - .mul(answerLiquidity?.price.price ?? 0) - .toFixed(1)}{" "} + {new Decimal(amount || 0).mul(price || 0).toFixed(1)}{" "} <span className="text-ztg-primary-400">≈</span>{" "} - {baseAssetPrice && - answerLiquidity?.amount && - answerLiquidity?.price.price - ? baseAssetPrice - .mul(answerLiquidity.amount) - .mul(answerLiquidity.price.price) - .toFixed(2) - : "--"}{" "} + {baseAssetPrice + ? baseAssetPrice.mul(amount).mul(price).toFixed(2) + : "--"}{" "} USD </> ) : ( - "" + "--" )} </span> </div> <div className="flex justify-between"> <span className="text-ztg-primary-200">Price:</span> <span className="font-semibold text-ztg-primary-100"> - {new Decimal(answerLiquidity?.price.price ?? 0).toFixed(2)}{" "} - {baseCurrency} + {price !== undefined + ? `${new Decimal(price || 0).toFixed(2)} ${baseCurrency}` + : "--"} </span> </div> </div> ) : ( "" )} </div> ); })}
🧹 Nitpick comments (10)
components/create/editor/inputs/DateTime.tsx (1)
16-16: Unused prop:hasValueis never referenced.The
hasValueprop is declared in the interface and destructured in the component signature but is never used in the component body. Consider removing it if it's not needed, or implement the intended functionality.Apply this diff to remove the unused prop:
export type DateTimePickerProps = { name: string; value?: string; timezone?: string; onChange: (event: FormEvent<string>) => void; onBlur: (event: FormEvent<string>) => void; placeholder?: string; isValid?: boolean; - hasValue?: boolean; className?: string; }; export const DateTimePicker: React.FC<DateTimePickerProps> = ({ name, value, timezone, onChange, onBlur, placeholder, isValid, - hasValue, className, }) => {Also applies to: 28-28
components/create/editor/inputs/BlockPeriod.tsx (1)
261-263: Simplify redundant ternary operator.Both branches of the ternary return
text-white, making the conditional unnecessary.Apply this diff to simplify:
- className={`min-w-[70px] bg-transparent text-sm text-white outline-none ${ - isSelected ? "text-white" : "text-white" - }`} + className="min-w-[70px] bg-transparent text-sm text-white outline-none"components/markets/MarketsList.tsx (1)
79-79: Consider simplifying the key withitem.slug.While the type prefix now prevents the collision issue noted in the earlier review, the ternary expression can be replaced with the simpler
item.slug, which is already guaranteed unique and stable for each item type.Apply this diff:
- key={`${item.type}-${item.type === "market" ? item.data.marketId : item.data.poolId}`} + key={item.slug}components/account/AccountButton.tsx (2)
66-70: Optional chaining won't prevent runtime error.Line 67 uses
balance?.div(ZTG).abs().toNumber()but optional chaining only protects the first call. Ifbalanceis defined but.div()returnsundefinedor.abs()returnsundefined, the subsequent chaining will throw. The current guard at line 66 (balance &&) correctly prevents this, making the optional chaining on line 67 redundant.Consider simplifying to:
- {balance && - `${formatNumberLocalized(balance?.div(ZTG).abs().toNumber())} ${ + {balance && + `${formatNumberLocalized(balance.div(ZTG).abs().toNumber())} ${ units ?? "" }`}
76-100: Consider improving type safety for AccountMenuContentProps.Seven props use
anytype (activeAccount, avatarDeps, alerts, identity, proxy, accountModals, constants), which bypasses TypeScript's type checking. While this may be acceptable for a "Chill" review, consider importing proper types from the relevant modules for better maintainability and IDE support.components/account/NotificationsPanel.tsx (1)
40-44: Remove unused hook call and destructured value.Line 41:
useMobileViewport()is called but based on the relevant code snippet (lib/hooks/useMobileViewport.ts), this is an empty function that does nothing. It can be removed.Line 44:
isSwipingis destructured fromuseMenuSwipeToClosebut never used in the component.Apply this diff:
- // Initialize mobile viewport handling - useMobileViewport(); - // Enable swipe-to-close on mobile (swipe left to close) - const { menuRef } = useMenuSwipeToClose(isOpen, onClose, "left"); + const { menuRef } = useMenuSwipeToClose(isOpen, onClose, "left");components/markets/MarketMetadataBadges.tsx (2)
89-102: Handle optional token prop for tooltip.Since
tokenis optional, the tooltip could display an empty or undefined value when token is not provided. Consider adding a fallback.Apply this diff:
label="Currency" - tooltip={token} + tooltip={token ?? "ZTG"} bgColor="bg-white/60"
214-219: Consider rendering MarketFavoriteToggle without IconBadge wrapper.
MarketFavoriteToggleis an interactive element (click to toggle), butIconBadgeappliescursor-helpstyling (line 40) which is appropriate for non-interactive badges. For consistency with other interactive badges (Trusted, Edited) and custom badges (CompactCreatorBadge), consider rendering it with its own background and tooltip implementation.Apply this diff:
- {/* Favorite Toggle - Icon Only */} - <IconBadge - icon={<MarketFavoriteToggle size={14} marketId={market.marketId} />} - tooltip="Toggle Favorite" - bgColor="bg-pink-100/80" - tooltipAlign="right" - /> + {/* Favorite Toggle */} + <div className="group relative"> + <div className="flex h-6 w-6 cursor-pointer items-center justify-center rounded-lg bg-pink-100/80 backdrop-blur-sm transition-all hover:scale-110 hover:opacity-95 hover:shadow-md"> + <MarketFavoriteToggle size={14} marketId={market.marketId} /> + </div> + <div className="pointer-events-none absolute bottom-full right-0 z-10 mb-1 whitespace-nowrap opacity-0 transition-opacity group-hover:opacity-100"> + <div className="rounded-md bg-gray-900 px-2 py-1 text-xs text-white shadow-lg"> + Toggle Favorite + </div> + </div> + </div>components/assets/AssetActionButtons/RedeemButton.tsx (2)
249-288: Extract duplicate validation logic into a helper function.The rectangular grid validation at lines 266-270 is duplicated at lines 327-331 in the extrinsic builder. Consider extracting this into a helper function to reduce duplication and ensure consistent error handling.
// Add near the top of the file const validateRectangularGrid = ( totalCombinations: number, numParentOutcomes: number ): number => { if (totalCombinations % numParentOutcomes !== 0) { console.error( `Invalid market structure: totalCombinations (${totalCombinations}) is not evenly divisible by numParentOutcomes (${numParentOutcomes})` ); } return Math.floor(totalCombinations / numParentOutcomes); };Then replace both occurrences with:
const numChildOutcomes = validateRectangularGrid(totalCombinations, numParentOutcomes);
339-344: Add explanatory comment for hard-codedtotal: 16.Line 343 uses
{ total: 16, consumeAll: true }without explanation. Past review comments show confusion about this value. Add a comment explaining why 16 is the correct fixed value (e.g., if the blockchain always uses 16-bit index representation) to prevent future misunderstanding.return sdk.api.tx.combinatorialTokens.redeemPosition( parentCollectionId, marketIds, tokenIndex, + // The blockchain uses a fixed 16-bit bitset for index representation regardless of outcome count { total: 16, consumeAll: true } );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
components/account/AccountButton.tsx(7 hunks)components/account/NotificationsPanel.tsx(1 hunks)components/assets/AssetActionButtons/RedeemButton.tsx(6 hunks)components/court/CourtVoteForm.tsx(4 hunks)components/court/CourtVoteRevealForm.tsx(2 hunks)components/create/editor/Summary.tsx(3 hunks)components/create/editor/inputs/BlockPeriod.tsx(2 hunks)components/create/editor/inputs/DateTime.tsx(4 hunks)components/create/editor/inputs/FeeSelect.tsx(3 hunks)components/create/editor/inputs/answers/Categorical.tsx(5 hunks)components/liquidity/PoolSettings.tsx(8 hunks)components/markets/MarketHistoryModal.tsx(1 hunks)components/markets/MarketMetadataBadges.tsx(1 hunks)components/markets/MarketsList.tsx(1 hunks)components/onboarding/DisclaimerModal.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- components/court/CourtVoteRevealForm.tsx
- components/markets/MarketHistoryModal.tsx
- components/court/CourtVoteForm.tsx
🧰 Additional context used
🧬 Code graph analysis (9)
components/onboarding/DisclaimerModal.tsx (1)
components/ui/ModalPanel.tsx (3)
ModalPanel(30-67)ModalHeader(72-89)ModalBody(94-114)
components/create/editor/inputs/BlockPeriod.tsx (1)
components/create/editor/inputs/DateTime.tsx (1)
DateTimePicker(20-112)
components/account/AccountButton.tsx (6)
lib/util/index.ts (2)
formatNumberLocalized(31-40)shortenAddress(4-10)lib/constants/index.ts (1)
ZTG(12-12)lib/state/alerts/useAlerts.ts (1)
useAlerts(38-118)lib/hooks/queries/useIdentity.ts (1)
useIdentity(8-73)lib/hooks/useSimpleScrollLock.ts (1)
useSimpleScrollLock(10-34)components/account/NotificationsPanel.tsx (1)
NotificationsPanel(30-176)
components/liquidity/PoolSettings.tsx (1)
components/ui/InfoPopover.tsx (1)
InfoPopover(22-114)
components/markets/MarketMetadataBadges.tsx (4)
components/markets/MarketFavoriteToggle.tsx (1)
MarketFavoriteToggle(4-29)components/markets/MarketHeaderUtils.tsx (2)
CompactCreatorBadge(49-65)CompactAddress(67-87)components/ui/InfoPopover.tsx (1)
InfoPopover(22-114)components/markets/PromotionCallout.tsx (1)
MarketPromotionCallout(10-106)
components/account/NotificationsPanel.tsx (8)
lib/state/wallet.tsx (1)
useWallet(485-612)lib/state/alerts/useAlerts.ts (1)
useAlerts(38-118)lib/hooks/useMobileViewport.ts (1)
useMobileViewport(3-3)lib/hooks/useSwipeGesture.ts (1)
useMenuSwipeToClose(106-123)lib/hooks/useSimpleScrollLock.ts (1)
useSimpleScrollLock(10-34)lib/hooks/queries/court/useCaseMarketId.ts (1)
useCaseMarketId(10-27)lib/hooks/queries/useMarket.ts (1)
useMarket(20-53)lib/state/alerts/types.ts (5)
CourtCaseReadyForVote(49-52)CourtCaseReadyForReveal(54-57)ReadyToReportMarketAlertData(34-37)RedeemableMarketsAlertData(44-47)RelevantMarketDisputeAlertData(39-42)
components/create/editor/Summary.tsx (2)
lib/util/index.ts (1)
shortenAddress(4-10)lib/state/market-creation/types/form.ts (1)
blocksAsDuration(219-221)
components/markets/MarketsList.tsx (4)
lib/hooks/queries/useInfiniteMarkets.ts (1)
useInfiniteMarkets(27-162)lib/hooks/queries/useInfiniteMultiMarkets.ts (1)
useInfiniteMultiMarkets(20-166)components/ui/Loader.tsx (1)
Loader(14-76)components/markets/market-card/MarketOrComboCard.tsx (1)
MarketOrComboCard(121-151)
components/assets/AssetActionButtons/RedeemButton.tsx (4)
lib/types/combinatorial.ts (2)
isCombinatorialToken(21-24)CombinatorialToken(16-16)lib/constants/index.ts (1)
ZTG(12-12)lib/types/virtual-market.ts (1)
VirtualMarket(7-13)lib/hooks/useExtrinsic.ts (1)
useExtrinsic(12-134)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Jest
🔇 Additional comments (27)
components/create/editor/inputs/DateTime.tsx (2)
92-92: Verify removal of seconds from date format.The date format was changed from
"MMM D, YYYY, h:mm:ss A"to"MMM D, YYYY, h:mm A", removing seconds precision from the displayed date. Confirm this is intentional and doesn't negatively impact use cases where second-level precision is expected.
63-66: LGTM: Styling modernization looks good.The styling updates consistently modernize the component across both Firefox and non-Firefox rendering paths with improved visual hierarchy, hover states, and glassmorphism effects using backdrop blur.
Also applies to: 82-93
components/liquidity/PoolSettings.tsx (6)
2-2: InfoPopover import correctly fixed.The import has been updated to use named export syntax, resolving the previous critical issue. This now correctly matches the export in
components/ui/InfoPopover.tsx.
117-117: LGTM: hideBaseLiquidity prop cleanly implemented.The optional
hideBaseLiquidityprop is well-typed with a sensible default value, allowing flexible control of the Base Liquidity section visibility.Also applies to: 126-126
78-106: LGTM: PriceSetter UI refinements improve layout and messaging.The component styling updates create a more compact, refined interface with clearer validation hints and better proportioned icons.
216-263: LGTM: Base Liquidity section correctly gated.The conditional rendering cleanly removes the entire Base Liquidity section from the DOM when
hideBaseLiquidityis true, supporting the combo markets use case mentioned in the PR objectives.
190-203: LGTM: Explicit column widths ensure consistent layout.The table columns now have defined percentage widths that sum to 100%, providing predictable spacing across different viewport sizes.
215-215: LGTM: Responsive wrapper ensures table accessibility on small screens.The full-width container and horizontal overflow handling prevent layout breaks on narrow viewports.
Also applies to: 265-271
components/create/editor/inputs/answers/Categorical.tsx (3)
124-126: LGTM!The Yes/No detection logic correctly identifies the mode based on disabled state and answer count, which aligns with the PR's objectives for handling Yes/No markets as read-only with exactly two answers.
208-218: LGTM! Clean Yes/No chip rendering.The compact read-only chip display for Yes/No mode is well-implemented and appropriately bypasses the full input controls since these markets are view-only.
235-245: LGTM! Accessibility issue resolved.The aria-label has been properly added to the remove button, addressing the previous review feedback. The label dynamically includes the answer value when available, making the button's purpose clear to screen reader users.
components/create/editor/inputs/FeeSelect.tsx (2)
141-145: Past issue partially addressed: NaN no longer renders, but state still holds invalid data.The display logic now correctly prevents
"NaN"from appearing in the input field (addressing the previous review comment). However, becausehandleCustomInputChangestoresNaNin state (line 90), this is a display-level workaround rather than a full fix. Consider the refactor suggested in lines 85-98 to avoid storing invalid data in the first place.
26-83: Verify the immediateonBlurcall pattern after every select change.Each selection path (custom, empty, preset) calls
onBlurimmediately afteronChange. This differs from typical blur behavior (which fires when focus leaves the element) and may trigger validation or side effects prematurely. Please confirm this is the intended behavior for dropdown selections in your form framework.components/create/editor/inputs/BlockPeriod.tsx (2)
125-138: LGTM: Clean preset button implementation.The individual preset button rendering with distinct active/inactive states is well-implemented. The conditional styling clearly indicates the selected preset.
140-152: Excellent fix: Custom duration input is now accessible.The removal of the
value?.type === "duration"condition resolves the accessibility issue flagged in previous reviews. Users can now access the custom duration input immediately without first selecting a preset. The default value fallback ensures the component always has valid data.components/onboarding/DisclaimerModal.tsx (1)
12-23: Previous review concern properly addressed.The setTimeout cleanup has been correctly implemented, resolving the memory leak issue flagged in the previous review. The timeout ID is now stored and cleared in the cleanup function, preventing any attempt to access the ref after unmount or when
modalOpenchanges.components/markets/MarketsList.tsx (1)
23-35: Conditional query enabling successfully implemented.The dual-query pattern now correctly uses the
enabledflag to prevent simultaneous API calls. Only the relevant query (useInfiniteMarketsoruseInfiniteMultiMarkets) executes based onisMultiMarket, eliminating the unnecessary API load flagged in the previous review.components/account/AccountButton.tsx (1)
102-137: Excellent fix for the Rules-of-Hooks violation.The extraction of
AccountMenuContentsuccessfully resolves the critical Rules-of-Hooks issue flagged in the previous review. TheuseEffecthooks are now correctly called at the component's top level rather than inside a render-prop callback.components/account/NotificationsPanel.tsx (1)
366-398: Empty destructuring issue resolved.The component now properly accepts and uses the
alertprop, resolving theno-empty-patternlint error flagged in the previous review.components/markets/MarketMetadataBadges.tsx (1)
16-21: LGTM! Dynamic import fix applied correctly.The named export extraction using
.then((m) => m.MarketFavoriteToggle)properly resolves the issue from the previous review.components/assets/AssetActionButtons/RedeemButton.tsx (7)
26-30: LGTM!The new imports for combinatorial token support are well-organized and necessary for the multi-market functionality.
33-62: LGTM!The prop extensions are backward-compatible (all optional) and well-documented with inline comments explaining their purpose in multi-market contexts.
197-203: LGTM!The
useMemodependencies correctly includeisPartialRedemption, which affects the balance calculation logic at line 122.
205-227: LGTM!The optional balance display is cleanly implemented with proper loading state handling and doesn't affect the default behavior when
showBalanceis false.
229-241: LGTM!Extending
assetIdto acceptCombinatorialTokenis a natural type union for multi-market support, and the additional props remain optional for backward compatibility.
352-371: LGTM!The success notification and UI updates appropriately distinguish between standard and multi-market redemptions, and the styled badge provides better visual feedback.
295-300: String-based combinatorial detection is stable and correct.The code uses a well-designed multi-signal approach:
- Single-market combinatorial:
asset.includes("combinatorialToken")detects literal string in outcomeAssets (stable—market construction sets this format)- Multi-market combinatorial:
market.marketType.categorical === "Combinatorial"provides explicit type check- Multi-market with underlying:
underlyingMarketIds && underlyingMarketIds.length > 0fallbackThe
outcomeAssetsfield is typed asstring[]throughout the codebase. Virtual markets are constructed to populate it with either the literal "combinatorialToken" string (single-market) or hex CombinatorialTokenString values (multi-market), making the detection approach reliable. The same pattern is used elsewhere in the same file (line 110–111) and passes safeguards during market construction.
| const RelevantMarketDisputeItem = ({ | ||
| alert, | ||
| }: { | ||
| alert: RelevantMarketDisputeAlertData; | ||
| }) => { | ||
| const router = useRouter(); | ||
|
|
||
| useEffect(() => { | ||
| router.prefetch(`/markets/${alert.market.marketId}`); | ||
| }, [alert, router]); | ||
|
|
||
| return ( | ||
| <AlertCard | ||
| onClick={() => { | ||
| router.push(`/markets/${alert.market.marketId}`); | ||
| }} | ||
| > | ||
| <div className="mb-2"> | ||
| <div className="inline-flex items-center gap-1.5 rounded-full bg-gradient-to-r from-red-500/30 to-orange-600/40 px-2.5 py-1 text-xs font-semibold backdrop-blur-sm"> | ||
| <span className="text-white">Market Dispute</span> | ||
| </div> | ||
| </div> | ||
| <div> | ||
| <h3 className="mb-1 text-sm font-semibold text-white"> | ||
| {alert.market.question} | ||
| </h3> | ||
| <p className="text-xs text-white/75"> | ||
| A market you're involved in is disputed | ||
| </p> | ||
| </div> | ||
| </AlertCard> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Overly broad useEffect dependencies.
Line 375: The dependency array [alert, router] should be narrowed to [alert.market.marketId, router] since only that specific property is used in the effect.
Apply this diff:
useEffect(() => {
router.prefetch(`/markets/${alert.market.marketId}`);
- }, [alert, router]);
+ }, [alert.market.marketId, router]);🤖 Prompt for AI Agents
In components/account/NotificationsPanel.tsx around lines 366 to 398 the
useEffect currently depends on the whole alert object and router; narrow the
dependency array to only the values actually used by the effect by replacing
[alert, router] with [alert.market.marketId, router] so the prefetch only
re-runs when the marketId or router changes.
| {market.disputeMechanism === "Court" && ( | ||
| <IconBadge | ||
| icon={ | ||
| <Image width={12} height={12} src="/icons/court.svg" alt="court" /> | ||
| } | ||
| tooltip="Court Dispute Mechanism" | ||
| bgColor="bg-purple-100/80" | ||
| /> | ||
| )} |
There was a problem hiding this comment.
🧩 Analysis chain
Verify disputeMechanism value coverage.
The logic renders a Court badge when disputeMechanism === "Court" and a Trusted badge when !market.disputeMechanism (falsy). If disputeMechanism can have other non-falsy values besides "Court", neither badge would render. Please verify all possible values are handled.
Run this script to check disputeMechanism usage patterns:
Also applies to: 130-163
🏁 Script executed:
#!/bin/bash
# Search for disputeMechanism assignments and checks
# Find disputeMechanism field definitions and type declarations
rg -n "disputeMechanism" --type=ts --type=tsx -A 2 -B 2
# Search for disputeMechanism enum or type definitions
ast-grep --pattern 'disputeMechanism: $$$'Length of output: 27
🏁 Script executed:
#!/bin/bash
# Search for disputeMechanism type definition and usage patterns
# Search for disputeMechanism in TypeScript/TSX files without specifying tsx type
rg -n "disputeMechanism" -A 3 -B 3 -g "*.ts" -g "*.tsx"
# Search for Market interface/type definition
rg -n "interface Market|type Market" -A 10 -g "*.ts" -g "*.tsx"
# Search for disputeMechanism enum or union type
rg -n "disputeMechanism.*=" -g "*.ts" -g "*.tsx"Length of output: 55185
Handle missing dispute mechanism values in badge logic.
The code checks for disputeMechanism === "Court" and !market.disputeMechanism, but the type definition supports three values: "SimpleDisputes" | "Authorized" | "Court" (lib/gql/markets.ts:134). If disputeMechanism is "Authorized" or "SimpleDisputes", neither badge renders. Update the Trusted badge condition to explicitly account for all non-Court values.
Also applies to: 130-163
🤖 Prompt for AI Agents
In components/markets/MarketMetadataBadges.tsx around lines 105-113 (and also
update the similar block at 130-163), the badge logic only shows the Court badge
when disputeMechanism === "Court" and otherwise skips showing a Trusted badge;
you must explicitly render the Trusted badge for the other supported enum
values. Replace the current Trusted condition with an explicit check for
market.disputeMechanism === "Authorized" || market.disputeMechanism ===
"SimpleDisputes" (and also treat undefined/null as Trusted if intended by code)
so Trusted renders for those two values (or when missing), and keep the Court
badge only for "Court"; apply the same condition changes to the other badge
block at lines 130-163.
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/create/editor/inputs/answers/Categorical.tsx (1)
100-111: Fix drag reorder guard for first answer.
findIndexreturns0for the first answer, but the guard treats0as falsy, so any drag involving the first item exits early and the reorder never happens. Switch the guard to explicit=== -1checks (and keep the existing!value?.answers), so index0is accepted while missing items still bail out.Apply this diff:
- if (!oldIndex || !newIndex || !value?.answers) return; + if ( + !value?.answers || + oldIndex === undefined || + newIndex === undefined || + oldIndex === -1 || + newIndex === -1 + ) { + return; + }
♻️ Duplicate comments (2)
components/account/NotificationsPanel.tsx (1)
308-311: Narrow the prefetch effect dependency array.
useEffectonly readsalert.market.marketId, so[alert, router]retriggers whenever thealertobject identity changes. Please depend just on the fields actually used, e.g.[alert.market.marketId, router], to avoid unnecessary prefetch churn.components/create/editor/inputs/LiquiditySimple.tsx (1)
52-54: Fix per-outcome liquidity amounts in simple mode.This is the same critical issue flagged in the previous review: Line 54 assigns the total liquidity
amountto every outcome row. When these amounts are summed by the extrinsic builder, a two-outcome market funded with "100" will incorrectly send "200" total.The Distribution Preview (lines 150-152) correctly multiplies by
ratio, but the actual data sent viaonChangedoes not, creating an inconsistency between what users see and what gets submitted.Apply this diff to multiply by the equal-share ratio before storing each row:
+ const perOutcomeAmount = new Decimal(amount || "0") + .mul(ratio) + .toString(); + return { asset: outcomeName, - amount: amount || "0", + amount: perOutcomeAmount, price: { price: new Decimal(ratio).toString(), locked: false, }, };
🧹 Nitpick comments (1)
components/create/editor/Summary.tsx (1)
80-86: Remove misleading non-null assertion on optional propLine 82 uses a non-null assertion (
baseAssetPrice!) when passing a potentially undefined value toAnswersDisplay. While the component accepts an optionalbaseAssetPrice?: Decimalparameter and handles undefined correctly, the assertion is misleading.<AnswersDisplay answers={form.answers!} - baseAssetPrice={baseAssetPrice!} + baseAssetPrice={baseAssetPrice} baseCurrency={form.currency!} liquidity={form?.liquidity} moderation={form.moderation!} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
components/account/NotificationsPanel.tsx(1 hunks)components/assets/AssetActionButtons/RedeemButton.tsx(7 hunks)components/create/editor/Summary.tsx(3 hunks)components/create/editor/inputs/FeeSelect.tsx(3 hunks)components/create/editor/inputs/LiquiditySimple.tsx(1 hunks)components/create/editor/inputs/LiquidityUnified.tsx(1 hunks)components/create/editor/inputs/answers/Categorical.tsx(5 hunks)components/markets/MarketMetadataBadges.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
components/create/editor/Summary.tsx (3)
lib/util/index.ts (1)
shortenAddress(4-10)lib/state/market-creation/types/form.ts (1)
blocksAsDuration(219-221)pages/markets/[marketid].tsx (1)
QuillViewer(113-118)
components/create/editor/inputs/LiquiditySimple.tsx (4)
lib/state/market-creation/types/form.ts (2)
Liquidity(114-114)CurrencyTag(96-96)lib/constants/supported-currencies.ts (1)
getMetadataForCurrency(43-44)lib/hooks/queries/useAssetUsdPrice.ts (1)
useAssetUsdPrice(19-40)components/create/editor/inputs/FeeSelect.tsx (1)
Fee(5-5)
components/create/editor/inputs/LiquidityUnified.tsx (6)
lib/state/market-creation/types/form.ts (3)
Liquidity(114-114)CurrencyTag(96-96)Answers(99-99)lib/state/market-creation/types/fieldstate.ts (1)
FieldState(7-20)lib/constants/supported-currencies.ts (1)
getMetadataForCurrency(43-44)lib/hooks/queries/useAssetUsdPrice.ts (1)
useAssetUsdPrice(19-40)components/create/editor/inputs/FeeSelect.tsx (1)
Fee(5-5)components/create/editor/inputs/Liquidity.tsx (1)
LiquidityInput(22-59)
components/markets/MarketMetadataBadges.tsx (5)
components/markets/MarketFavoriteToggle.tsx (1)
MarketFavoriteToggle(4-29)lib/gql/markets.ts (1)
MarketPageIndexedData(93-136)components/markets/MarketHeaderUtils.tsx (2)
CompactCreatorBadge(49-65)CompactAddress(67-87)components/ui/InfoPopover.tsx (1)
InfoPopover(22-114)components/markets/PromotionCallout.tsx (1)
MarketPromotionCallout(10-106)
components/assets/AssetActionButtons/RedeemButton.tsx (4)
lib/hooks/queries/useMarket.ts (1)
useMarket(20-53)lib/types/combinatorial.ts (2)
isCombinatorialToken(21-24)CombinatorialToken(16-16)lib/constants/index.ts (1)
ZTG(12-12)lib/types/virtual-market.ts (1)
VirtualMarket(7-13)
components/account/NotificationsPanel.tsx (8)
lib/state/wallet.tsx (1)
useWallet(485-612)lib/state/alerts/useAlerts.ts (1)
useAlerts(38-118)lib/hooks/useMobileViewport.ts (1)
useMobileViewport(3-3)lib/hooks/useSwipeGesture.ts (1)
useMenuSwipeToClose(106-123)lib/hooks/useSimpleScrollLock.ts (1)
useSimpleScrollLock(10-34)lib/hooks/queries/court/useCaseMarketId.ts (1)
useCaseMarketId(10-27)lib/hooks/queries/useMarket.ts (1)
useMarket(20-53)lib/state/alerts/types.ts (5)
CourtCaseReadyForVote(49-52)CourtCaseReadyForReveal(54-57)ReadyToReportMarketAlertData(34-37)RedeemableMarketsAlertData(44-47)RelevantMarketDisputeAlertData(39-42)
🔇 Additional comments (9)
components/create/editor/Summary.tsx (9)
57-70: LGTM: Clean card-based layoutThe Question section restructuring improves visual hierarchy with consistent styling and appropriate placeholder handling.
139-141: Good defensive null checkingThe explicit
!= nullchecks for bothbaseAssetPriceandbaseAmountcorrectly distinguish null/undefined from legitimate zero values before calling numeric methods.
178-190: LGTM: Clean moderation displayThe card-based layout with optional dispute mechanism rendering is well-structured.
192-201: LGTM: Responsive oracle address displayThe responsive address shortening for mobile viewports is appropriate.
203-237: LGTM: Improved timeline layoutThe two-column grid layout for periods improves readability while maintaining proper fallback handling.
239-251: LGTM: Description renderingThe QuillViewer integration with appropriate placeholder fallback is well-implemented.
253-259: LGTM: Creator fee displayClean and straightforward fee display.
318-326: Well-guarded USD value calculationThe defensive checks for
baseAssetPrice,answerLiquidity?.amount, andanswerLiquidity?.price.pricebefore chaining.mul()and.toFixed()correctly prevent runtime errors when price data is loading or missing.
351-360: LGTM: Simple label componentThe Label component provides consistent styling with flexible class name composition.
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (2)
components/create/editor/inputs/LiquiditySimple.tsx (1)
50-85: Critical: Per-outcome liquidity amounts are still incorrect.Line 65 assigns the total liquidity amount to each outcome row, which causes the extrinsic builder to sum all rows and deploy with
numOutcomes × amountinstead of the intendedamount. For example, entering "100" with 2 outcomes will deploy "200" total liquidity.This issue was flagged in a previous review but remains unfixed. Additionally, the distribution preview (lines 163-165) correctly multiplies by
ratio, creating an inconsistency between what users see and what gets stored.Apply this diff to fix the per-outcome calculation:
+ const perOutcomeAmount = new Decimal(amount || "0") + .mul(ratio) + .toString(); + return { asset: outcomeName, - amount: amount || "0", + amount: perOutcomeAmount, price: {components/create/editor/Summary.tsx (1)
319-341: Fix nested optional access before formatting liquidity values
answerLiquidity?.price.pricestill dereferences.priceonundefined, so a liquidity row missing the nestedpriceobject (e.g., while data loads or when the API omits it) throwsTypeError: Cannot read properties of undefined (reading 'price'), breaking the summary panel. Please extend the optional chaining to the nested field everywhere we touch it in this block.- {new Decimal(answerLiquidity?.amount || 0) - .mul(answerLiquidity?.price.price ?? 0) + {new Decimal(answerLiquidity?.amount || 0) + .mul(answerLiquidity?.price?.price ?? 0) .toFixed(1)}{" "} <span className="text-ztg-primary-400">≈</span>{" "} - {baseAssetPrice && - answerLiquidity?.amount && - answerLiquidity?.price.price + {baseAssetPrice && + answerLiquidity?.amount && + answerLiquidity?.price?.price ? baseAssetPrice .mul(answerLiquidity.amount) - .mul(answerLiquidity.price.price) + .mul(answerLiquidity.price?.price) .toFixed(2) : "--"}{" "} USD ... - {new Decimal(answerLiquidity?.price.price ?? 0).toFixed(2)}{" "} + {new Decimal(answerLiquidity?.price?.price ?? 0).toFixed(2)}{" "}
🧹 Nitpick comments (3)
components/create/editor/inputs/LiquidityUnified.tsx (2)
43-46: Consider adding a TODO or removing commented code.The stablecoin logic is "temporarily disabled" but the commented code remains. If re-enabling is planned, add a TODO with context. Otherwise, remove the dead code to keep the file clean.
92-92: Type assertion toanyreduces type safety.Casting
rowstoanybypasses TypeScript checks. If the shape is well-defined, consider typing the rows array explicitly or ensuring the constructed objects match the expected type.components/assets/AssetActionButtons/RedeemButton.tsx (1)
182-196: Consider renaming the shadowed variable for clarity.Line 186 redeclares
numChildOutcomeswithin theisChildScalar && !isParentScalarblock, shadowing the variable calculated earlier (lines 130-149). While the hard-coded value of2is correct for scalar markets, the shadowing may confuse future maintainers or hide bugs during refactoring.Consider renaming to make the local scope explicit:
if (isChildScalar && !isParentScalar) { // Parent categorical, child scalar // Check if this position's parent outcome matches the resolved parent const parentResolvedIndex = Number(market.resolvedOutcome); - const numChildOutcomes = 2; // Scalar has 2 outcomes - const parentIndex = Math.floor(tokenIndex / numChildOutcomes); + const scalarOutcomeCount = 2; // Scalar always has Long/Short outcomes + const parentIndex = Math.floor(tokenIndex / scalarOutcomeCount);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
components/assets/AssetActionButtons/RedeemButton.tsx(7 hunks)components/create/editor/Summary.tsx(3 hunks)components/create/editor/inputs/LiquiditySimple.tsx(1 hunks)components/create/editor/inputs/LiquidityUnified.tsx(1 hunks)components/markets/MarketMetadataBadges.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
components/create/editor/inputs/LiquidityUnified.tsx (6)
lib/state/market-creation/types/form.ts (3)
Liquidity(114-114)CurrencyTag(96-96)Answers(99-99)lib/state/market-creation/types/fieldstate.ts (1)
FieldState(7-20)lib/constants/supported-currencies.ts (1)
getMetadataForCurrency(43-44)lib/hooks/queries/useAssetUsdPrice.ts (1)
useAssetUsdPrice(19-40)components/create/editor/inputs/FeeSelect.tsx (1)
Fee(5-5)components/create/editor/inputs/Liquidity.tsx (1)
LiquidityInput(22-59)
components/create/editor/Summary.tsx (3)
lib/util/index.ts (1)
shortenAddress(4-10)lib/state/market-creation/types/form.ts (1)
blocksAsDuration(219-221)pages/markets/[marketid].tsx (1)
QuillViewer(113-118)
components/create/editor/inputs/LiquiditySimple.tsx (4)
lib/state/market-creation/types/form.ts (2)
Liquidity(114-114)CurrencyTag(96-96)lib/constants/supported-currencies.ts (1)
getMetadataForCurrency(43-44)lib/hooks/queries/useAssetUsdPrice.ts (1)
useAssetUsdPrice(19-40)components/create/editor/inputs/FeeSelect.tsx (1)
Fee(5-5)
components/markets/MarketMetadataBadges.tsx (5)
components/markets/MarketFavoriteToggle.tsx (1)
MarketFavoriteToggle(4-29)lib/gql/markets.ts (1)
MarketPageIndexedData(93-136)components/markets/MarketHeaderUtils.tsx (2)
CompactCreatorBadge(49-65)CompactAddress(67-87)components/ui/InfoPopover.tsx (1)
InfoPopover(22-114)components/markets/PromotionCallout.tsx (1)
MarketPromotionCallout(10-106)
components/assets/AssetActionButtons/RedeemButton.tsx (4)
lib/hooks/queries/useMarket.ts (1)
useMarket(20-53)lib/types/combinatorial.ts (2)
isCombinatorialToken(21-24)CombinatorialToken(16-16)lib/constants/index.ts (1)
ZTG(12-12)lib/types/virtual-market.ts (1)
VirtualMarket(7-13)
🔇 Additional comments (8)
components/create/editor/inputs/LiquiditySimple.tsx (1)
32-36: Good defensive handling for undefined values.The code properly addresses previous concerns about undefined handling:
baseAssetPricehas a safe fallback chain (lines 32-36)defaultLiquidityprovides complete defaults (lines 43-48)- Both change handlers use
value ?? defaultLiquidityfor safety (lines 73, 88)totalValueincludes null coalescing (line 102)These improvements prevent runtime errors when props are undefined.
Also applies to: 43-48, 73-73, 88-88, 101-103
components/markets/MarketMetadataBadges.tsx (2)
1-21: Excellent fixes to the import issues!Both previously flagged critical import issues have been correctly resolved:
InfoPopovernow uses the named import syntaxMarketFavoriteTogglecorrectly extracts the named export via.then(m => m.MarketFavoriteToggle)These fixes ensure both components will render properly at runtime.
76-91: Well done on the dispute mechanism handling!The
getDisputeMechanismDescriptionfunction now properly handles all possible dispute mechanism values (Authorized,SimpleDisputes,Court, and absent/null), and the mechanism-specific copy provides clear, accurate descriptions for each type. This resolves the previously flagged issue about incomplete value coverage.components/create/editor/inputs/LiquidityUnified.tsx (1)
54-64: Well done resolving the undefined spread issue.The
basederivation now safely handles the case where bothinput.valueandvalueare undefined, preventing theTypeErrorflagged in previous reviews. The default structure matches the expectedLiquidityshape.components/assets/AssetActionButtons/RedeemButton.tsx (4)
86-92: LGTM: Safe child market fetching for multi-markets.The length check before accessing
underlyingMarketIds[1]prevents undefined access, and the optional pattern inuseMarketcorrectly handles the case wherechildMarketIdis undefined.
287-326: LGTM: Well-structured tokenIndex calculation with proper validation.The rectangular grid validation (lines 304-308) addresses the potential non-integer division issue raised in past reviews. The logic correctly handles both single-market and multi-market combinatorial tokens, with clear comments explaining the index calculation formula.
404-409: LGTM: Excellent UX improvement for redemption success state.The new styled badge with visual indicators (color dot, shadow, backdrop blur) provides much clearer feedback than plain text, enhancing the user experience for successful redemptions.
126-154: Acknowledge valid concern about multi-market child fetch failures, but implementation appears intentional.The
useMarkethook silently returnsnull/undefinedfor missing markets without error signaling. For multi-market scenarios, if the child market isn't found in the indexer, the fallback tomarket.categories.lengthwill yield the product (parent × child outcomes) rather than just child outcomes, breaking the calculation.The code structure suggests this fallback chain was intentional, but there's no mechanism to distinguish between:
- Single-market combos (childMarket undefined by design) → fallback is correct
- Multi-market with failed fetch (childMarket undefined unexpectedly) → fallback is wrong
The risk is silent failure: if a multi-market's child market is missing from the indexer, calculations will silently use incorrect values without warning.
Recommend either:
- Verify indexer reliability: Confirm child markets for valid multi-markets are always present in indexer
- Add explicit detection: Log or handle cases where
childMarketIdexists butchildMarketis still undefined after query completes- Document assumption: Add comment clarifying that fallback assumes child market presence for valid multi-market scenarios
Summary by CodeRabbit
New Features
Improvements
Documentation