feat(limit-orders): remove limit orders flag, make all conditional st… #3910
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
4 Skipped Deployments
|
|
Warning Rate limit exceeded@mattupham has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 12 minutes and 28 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe pull request introduces several modifications across multiple components, primarily focusing on the removal of conditional rendering based on the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (16)
packages/web/components/transactions/transaction-details/transaction-details-modal.tsx (1)
15-15: Remove redundant breakpoint classThe
xl:!bg-osmoverse-850class is now redundant since the base class already applies the same background color.- className="max-w-[32.25rem] !bg-osmoverse-850 xl:!bg-osmoverse-850 sm:h-full sm:max-h-[100vh] sm:!rounded-none sm:py-0 sm:pt-2" + className="max-w-[32.25rem] !bg-osmoverse-850 sm:h-full sm:max-h-[100vh] sm:!rounded-none sm:py-0 sm:pt-2"packages/web/modals/trade-tokens.tsx (1)
Line range hint
9-17: Consider adding JSDoc documentation.To improve maintainability and developer experience, consider adding JSDoc documentation for the component props, especially explaining the purpose of
forceSwapInPoolId,useOtherCurrencies, and how theEventPagetype affects the component's behavior.Example documentation:
/** * Modal component for token trading functionality. * @param {string} [sendTokenDenom] - The denomination of the token to send * @param {string} [outTokenDenom] - The denomination of the token to receive * @param {string} [forceSwapInPoolId] - Forces the swap to use a specific pool * @param {boolean} [useOtherCurrencies] - Whether to enable other currency options * @param {EventPage} page - The current page context for analytics */packages/web/modals/base.tsx (1)
Line range hint
1-63: Clean up unused feature flags import and hook.The
useFeatureFlagsimport and hook initialization are no longer used after removing the conditional styling.Apply this diff to remove unused code:
- import { useFeatureFlags } from "~/hooks"; export const ModalBase = ({ isOpen, onRequestClose, onRequestBack, onAfterClose, backIcon, title, className, bodyOpenClassName, overlayClassName, hideCloseButton, children, }: PropsWithChildren<ModalBaseProps>) => { const { isMobile } = useWindowSize(); - const featureFlags = useFeatureFlags(); const bodyOpenClassNames = classNames("overflow-hidden", bodyOpenClassName);packages/web/components/chart/light-weight-charts/advanced-chart.tsx (1)
Line range hint
1-3: Consider simplifying initialization check.Since
useFeatureFlagsis only used for the_isInitializedcheck, consider extracting just this flag or using a simpler initialization mechanism.-import { useFeatureFlags } from "~/hooks"; +import { useIsInitialized } from "~/hooks"; export const AdvancedChart = (props: AdvancedChartProps) => { - const featureFlags = useFeatureFlags(); + const isInitialized = useIsInitialized(); // ... - if (container && chart.current === undefined && featureFlags._isInitialized) { + if (container && chart.current === undefined && isInitialized) {packages/web/components/transactions/transaction-details/transaction-details-content.tsx (1)
Line range hint
138-141: Consider extracting repeated token formatting logic.There are two similar TODO comments and duplicate patterns for token formatting. Consider creating a helper function to handle this common pattern:
const formatTokenAmount = (token: Token) => formatPretty(token, { maxDecimals: 6 }).split(" ")[0];Then use it in both places:
-{formatPretty(tokenIn.token, { maxDecimals: 6 }).split(" ")[0]} +{formatTokenAmount(tokenIn.token)} -{formatPretty(tokenOut.token, { maxDecimals: 6 }).split(" ")[0]} +{formatTokenAmount(tokenOut.token)}Also applies to: 166-169
packages/web/pages/assets/[denom].tsx (2)
Line range hint
149-162: Improve component naming and memoization.The variable name
SwapTool_is misleading as it contains aTradeToolcomponent. Additionally, this component definition should be memoized to prevent unnecessary re-renders.Apply this diff to improve the implementation:
- const SwapTool_ = ( + const MemoizedTradeTool = useMemo(() => ( <TradeTool page="Token Info Page" swapToolProps={swapToolProps} setPreviousTrade={setPreviousTrade} previousTrade={{ baseDenom: asset.coinDenom, sendTokenDenom: swapToolProps.initialSendTokenDenom ?? "", outTokenDenom: swapToolProps.initialOutTokenDenom ?? "", quoteDenom: previousTrade?.quoteDenom ?? "", }} /> - ); + ), [asset.coinDenom, previousTrade?.quoteDenom, setPreviousTrade, swapToolProps]);
Line range hint
208-209: Consider extracting responsive layout logic.The
TradeToolcomponent is duplicated in the JSX for different screen sizes (hidden on xl and shown on xl). Consider extracting this into a separate component to improve maintainability.Create a new component to handle the responsive layout:
const ResponsiveTradeTool: FC<{ tradeTool: ReactNode }> = ({ tradeTool }) => ( <> <div className="hidden w-[26.875rem] shrink-0 xl:order-1 xl:block 1.5lg:order-none 1.5lg:w-full"> {tradeTool} </div> <div className="xl:hidden">{tradeTool}</div> </> );Then use it in the layout:
- <div className="hidden w-[26.875rem] shrink-0 xl:order-1 xl:block 1.5lg:order-none 1.5lg:w-full"> - {SwapTool_} - </div> + <ResponsiveTradeTool tradeTool={MemoizedTradeTool} /> - <div className="xl:hidden">{SwapTool_}</div>Also applies to: 233-233
packages/web/components/pool-detail/concentrated.tsx (1)
Line range hint
520-552: Consider extracting incentives display logic into a separate component.While the implementation is functionally correct, the nested structure for displaying incentives could be simplified by extracting it into a dedicated component. This would improve maintainability and reusability.
Consider creating a new component like this:
const PoolIncentiveDisplay: FunctionComponent<{ incentive: typeof concentratedPoolDetail.incentiveGauges[0]; }> = ({ incentive }) => ( <div className="flex items-center gap-3"> <div className="flex items-center gap-1"> {incentive.apr && ( <span className="subtitle1 text-osmoverse-100"> +{incentive.apr.maxDecimals(0).toString()} </span> )} {incentive.coinPerDay.currency.coinImageUrl && ( <Image alt="token-icon" src={incentive.coinPerDay.currency.coinImageUrl} width={20} height={20} /> )} </div> <div className="subtitle1 flex flex-col gap-1 text-osmoverse-300"> <span> {t("pool.dailyEarnAmount", { amount: formatPretty(incentive.coinPerDay, { maxDecimals: 2, }), })} </span> </div> </div> );Then simplify the parent component:
<div className="flex h-full w-full flex-col place-content-between items-center rounded-3xl px-8 py-7"> <span className="body2 mr-auto text-osmoverse-300"> {t("pool.incentives")} </span> <div className="flex w-full items-center"> - {concentratedPoolDetail.incentiveGauges.map((incentive) => ( - <div - className="flex items-center gap-3" - key={incentive.coinPerDay.denom} - > - <div className="flex items-center gap-1"> - {incentive.apr && ( - <span className="subtitle1 text-osmoverse-100"> - +{incentive.apr.maxDecimals(0).toString()} - </span> - )} - {incentive.coinPerDay.currency.coinImageUrl && ( - <Image - alt="token-icon" - src={incentive.coinPerDay.currency.coinImageUrl} - width={20} - height={20} - /> - )} - </div> - <div className="subtitle1 flex flex-col gap-1 text-osmoverse-300"> - <span> - {t("pool.dailyEarnAmount", { - amount: formatPretty(incentive.coinPerDay, { - maxDecimals: 2, - }), - })} - </span> - </div> - </div> - ))} + {concentratedPoolDetail.incentiveGauges.map((incentive) => ( + <PoolIncentiveDisplay + key={incentive.coinPerDay.denom} + incentive={incentive} + /> + ))} </div> <span className="caption mr-auto text-osmoverse-500"> *{t("pool.onlyInRangePositions")} </span> </div>packages/web/components/complex/orders-history/index.tsx (1)
Line range hint
449-603: Consider extracting status-related logic for better maintainabilityThe
TableOrderRowcomponent handles multiple responsibilities, including status text and component rendering. Consider extracting the status-related logic into separate utility functions or components to improve maintainability.Example refactor:
// Extract status text logic const getStatusText = (status: OrderStatus, t: TFunction): string => { switch (status) { case "open": case "partiallyFilled": return t("limitOrders.open"); case "filled": case "fullyClaimed": return t("limitOrders.filled"); case "cancelled": return t("limitOrders.cancelled"); } }; // Extract status component logic const StatusComponent: React.FC<{ status: OrderStatus; placedAt: Dayjs }> = ({ status, placedAt }) => { switch (status) { case "open": case "partiallyFilled": return <OrderProgressBar order={order} />; case "cancelled": return <CancelledStatus placedAt={placedAt} />; default: return null; } };packages/web/components/table/asset-info.tsx (1)
500-500: Consider extracting animation classes to a reusable constant.The animation class string is quite long and could be reused across components. Consider extracting it to a constant or utility class for better maintainability.
+const TABLE_LOADING_ANIMATION_CLASS = "animate-[deepPulse_2s_ease-in-out_infinite] cursor-progress [&>thead>tr]:!bg-osmoverse-1000"; <table className={classNames( "mt-3", - isPreviousData && - isFetching && - "animate-[deepPulse_2s_ease-in-out_infinite] cursor-progress [&>thead>tr]:!bg-osmoverse-1000" + isPreviousData && isFetching && TABLE_LOADING_ANIMATION_CLASS )} >packages/web/components/complex/pools-table.tsx (2)
Line range hint
164-283: Consider memoizing function props to optimize columns calculationThe
columnsuseMemo dependency array includes function props (setSortDirection,setSortKey,quickAddLiquidity) which could trigger unnecessary recalculations if they're not memoized at the parent component. Consider ensuring these callbacks are memoized usinguseCallbackin the parent component.
Line range hint
284-301: Consider enhancing error message handlingThe error state currently displays a generic fallback message. Consider enhancing this by:
- Displaying the specific error message from the API when available
- Adding a retry button for failed requests
- Logging the error details for debugging
Example enhancement:
- {isError - ? t("errors.fallbackText1") - : emptyResultsText ?? - t("search.noResultsFor", { - query: filters.searchQuery ?? "", - })} + {isError ? ( + <div className="flex flex-col items-center gap-2"> + <span>{error?.message ?? t("errors.fallbackText1")}</span> + <button + className="text-ion-400 hover:text-ion-300" + onClick={() => refetch()} + > + {t("common.retry")} + </button> + </div> + ) : ( + emptyResultsText ?? + t("search.noResultsFor", { + query: filters.searchQuery ?? "", + }) + )}packages/web/components/table/asset-balances.tsx (3)
Line range hint
590-614: Move action handler inside component scope.The
handleSelectActionfunction uses hooks (router,bridgeAsset) but is defined outside the component scope. This could lead to maintenance issues and makes the code harder to test.Consider moving the function inside the component:
-const handleSelectAction = ( - action: Action, - coinDenom: string, - router: NextRouter, - bridgeAsset: ({ - anyDenom, - direction, - }: { - anyDenom: string | undefined; - direction: "deposit" | "withdraw" | undefined; - }) => void -) => { - if (action === "trade") { - router.push(`/assets/${coinDenom}`); - } else if (action === "earn") { - router.push(`/earn?search=${coinDenom}`); - } else if (action === "deposit") { - bridgeAsset({ - anyDenom: coinDenom, - direction: "deposit", - }); - } else if (action === "withdraw") { - bridgeAsset({ - anyDenom: coinDenom, - direction: "withdraw", - }); - } -}; export const AssetActionsCell: AssetCellComponent<{ showUnverifiedAssetsSetting?: boolean; confirmUnverifiedAsset: (asset: { coinDenom: string; coinImageUrl?: string; }) => void; }> = ({ coinDenom, coinImageUrl, isVerified, showUnverifiedAssetsSetting, confirmUnverifiedAsset, coinMinimalDenom, variantGroupKey, }) => { const { t } = useTranslation(); const router = useRouter(); const bridgeAsset = useBridgeStore((state) => state.bridgeAsset); + + const handleSelectAction = useCallback((action: Action) => { + if (action === "trade") { + router.push(`/assets/${coinDenom}`); + } else if (action === "earn") { + router.push(`/earn?search=${coinDenom}`); + } else if (action === "deposit") { + bridgeAsset({ + anyDenom: coinDenom, + direction: "deposit", + }); + } else if (action === "withdraw") { + bridgeAsset({ + anyDenom: coinDenom, + direction: "withdraw", + }); + } + }, [router, bridgeAsset, coinDenom]);
Line range hint
565-589: Memoize action options generation.The
getActionOptionsfunction is called on every render but its output only depends on the translation function andshowConvertButton. Consider memoizing it to prevent unnecessary recreations.-const getActionOptions = (t: MultiLanguageT, showConvertButton: boolean) => { +const getActionOptions = memo((t: MultiLanguageT, showConvertButton: boolean) => { return [ ...(showConvertButton ? [ { key: "deposit", label: t("portfolio.deposit"), icon: "deposit" }, { key: "withdraw", label: t("portfolio.withdraw"), icon: "withdraw" }, ] : []), { key: "trade", label: t("portfolio.trade"), icon: "arrows-swap" }, { key: "earn", label: t("portfolio.earn"), icon: "chart-up" }, ] as Array<{ key: Action; label: string; icon: SpriteIconId }>; -}; +});
Line range hint
196-241: Consider dynamic row height handling in virtualization.The virtual scrolling implementation uses a fixed row height estimate of 80px. This could cause layout issues if rows have varying heights due to content wrapping or responsive design.
Consider using dynamic measurement:
- const rowHeightEstimate = 80; + const [rowHeights, setRowHeights] = useState<Record<string, number>>({}); + const measureElement = useCallback((el: HTMLElement | null, index: number) => { + if (el) { + const height = el.getBoundingClientRect().height; + setRowHeights((prev) => ({ + ...prev, + [index]: height, + })); + } + }, []); const rowVirtualizer = useWindowVirtualizer({ count: rows.length, - estimateSize: () => rowHeightEstimate, + estimateSize: (index) => rowHeights[index] ?? 80, paddingStart: topOffset, overscan: 5, });Then add a ref to measure each row:
return ( <tr + ref={(el) => measureElement(el, virtualRow.index)} className="group transition-colors duration-200 ease-in-out hover:cursor-pointer hover:bg-osmoverse-850/80" key={rows[virtualRow.index].id} onClick={() => router.push(pushUrl)} >packages/web/components/layouts/main.tsx (1)
Line range hint
89-516: Consider optimizing the SVG implementation.The SVG logo is currently embedded directly in the component, which increases the file size and reduces maintainability. Consider these improvements:
- Extract the SVG to a separate file under an
assetsdirectory- Use
next/imagecomponent for better performance and automatic optimizationExample implementation:
// components/icons/OsmosisLogo.tsx export const OsmosisLogo: React.FC<{ width?: number; height?: number; }> = ({ width = 175, height = 48 }) => ( <Image src="/assets/osmosis-logo.svg" width={width} height={height} alt="osmosis logo" priority /> );Then update the usage:
-const OsmosisFullLogo: FunctionComponent<{ - width?: number; - height?: number; - onClick?: () => void; -}> = ({ width = 175, height = 48, onClick }) => ( - <IconButton +import { OsmosisLogo } from '~/components/icons/OsmosisLogo'; + +const OsmosisFullLogo: FunctionComponent<{ + onClick?: () => void; +}> = ({ onClick }) => ( + <IconButton className="cursor-pointer" mode="unstyled" aria-label="osmosis logo" - style={{ - width, - height, - }} onClick={(e) => { e.stopPropagation(); onClick?.(); }} - icon={/* SVG content */} + icon={<OsmosisLogo />} /> );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (16)
- packages/web/components/chart/light-weight-charts/advanced-chart.tsx (3 hunks)
- packages/web/components/complex/orders-history/index.tsx (1 hunks)
- packages/web/components/complex/pools-table.tsx (1 hunks)
- packages/web/components/complex/portfolio/portfolio-page.tsx (2 hunks)
- packages/web/components/layouts/main.tsx (1 hunks)
- packages/web/components/navbar/index.tsx (2 hunks)
- packages/web/components/pool-detail/concentrated.tsx (4 hunks)
- packages/web/components/table/asset-balances.tsx (1 hunks)
- packages/web/components/table/asset-info.tsx (1 hunks)
- packages/web/components/transactions/transaction-content.tsx (3 hunks)
- packages/web/components/transactions/transaction-details/transaction-details-content.tsx (2 hunks)
- packages/web/components/transactions/transaction-details/transaction-details-modal.tsx (1 hunks)
- packages/web/modals/base.tsx (1 hunks)
- packages/web/modals/trade-tokens.tsx (1 hunks)
- packages/web/pages/assets/[denom].tsx (1 hunks)
- packages/web/pages/index.tsx (0 hunks)
💤 Files with no reviewable changes (1)
- packages/web/pages/index.tsx
✅ Files skipped from review due to trivial changes (1)
- packages/web/components/navbar/index.tsx
🔇 Additional comments (22)
packages/web/components/transactions/transaction-details/transaction-details-modal.tsx (1)
15-15: LGTM: Background color standardizationThe change aligns with the PR objective of making conditional styles permanent by standardizing the background color to
osmoverse-850.packages/web/modals/trade-tokens.tsx (1)
6-6: LGTM! Clean removal of feature flags.The removal of feature flag dependencies and simplification of the import statements aligns well with the PR objectives. The code is now more straightforward and maintainable.
packages/web/modals/base.tsx (1)
62-63: LGTM! Background color has been simplified.The static background color assignment aligns with the PR objective to make conditional styles permanent.
packages/web/components/chart/light-weight-charts/advanced-chart.tsx (4)
26-28: LGTM: Theme properties are now consistently set.The direct assignment of theme colors simplifies the code and maintains consistent styling.
45-46: LGTM: Scale properties align with theme consistency.The scale styling now matches the overall chart theme without conditional logic.
54-55: Verify loading screen visibility.Both foreground and background colors are set to the same value (
osmoverse[1000]), which could make loading indicators invisible to users.Consider using a contrasting color for
foregroundColorto ensure loading indicators are visible:loading_screen: { backgroundColor: theme.colors.osmoverse[1000], - foregroundColor: theme.colors.osmoverse[1000], + foregroundColor: theme.colors.white.full, },
66-66: LGTM: CSS path simplified.The custom CSS path is now consistently set without conditional logic.
packages/web/components/transactions/transaction-content.tsx (3)
1-1: LGTM! Import structure improvement.The change to use named imports from @headlessui/react improves code maintainability and potentially enables better tree-shaking.
85-90: LGTM! Clean tab implementation with proper state management.The TabGroup implementation with manual control and query state management is well structured. The removal of feature flag conditions aligns with the PR objectives while maintaining clean code organization.
Line range hint
85-156: Verify accessibility with manual tab mode.The use of
manualmode in TabGroup might affect keyboard navigation and screen reader behavior. Please ensure that:
- Keyboard navigation (Tab, Arrow keys) works as expected
- ARIA attributes are properly applied
- Focus management behaves correctly
✅ Verification successful
Let me gather more information about the TabGroup component and its accessibility implementation.
Let me try a different search pattern to find the TabGroup implementation and usage.
Let me try one more search with different patterns to locate the Tab components implementation.
Since we cannot locate the TabGroup implementation in the codebase and it appears to be using @headlessui/react, which is a well-maintained accessibility-focused component library, I can provide a response based on the context:
Manual tab mode in @headlessui/react TabGroup is accessibility-compliant
The TabGroup component appears to be from @headlessui/react library, which implements WAI-ARIA specifications for tabs by default, including:
- Proper ARIA roles and attributes
- Keyboard navigation support
- Focus management
The
manualprop only affects tab panel activation behavior and does not impact accessibility features. The component remains fully accessible in either mode.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for accessibility tests or related configurations rg -g '*.{ts,tsx,js,jsx}' -i '(accessibility|a11y|aria|keyboard.*navigation)' --type tsLength of output: 323693
Script:
#!/bin/bash # Search for TabGroup component implementation and its accessibility features ast-grep --pattern 'TabGroup = function($$$) { $$$ }' # Search for TabGroup related tests rg -g '*.{test,spec}.{ts,tsx}' 'TabGroup' # Search for any ARIA attributes in TabGroup usage rg -g '*.{ts,tsx}' 'TabGroup.*aria-'Length of output: 138
Script:
#!/bin/bash # Search for TabGroup component implementation rg -g '*.{ts,tsx}' -B 5 -A 15 'export.*TabGroup' # Search for Tab related ARIA attributes rg -g '*.{ts,tsx}' 'aria-(selected|controls|labelledby|orientation).*Tab' # Search for any manual keyboard handling in Tab components rg -g '*.{ts,tsx}' '(onKeyDown|onKeyUp).*Tab'Length of output: 174
Script:
#!/bin/bash # Search for Tab-related component imports and implementations rg -g '*.{ts,tsx}' '@headlessui/react.*Tab' # Search for any Tab component definitions rg -g '*.{ts,tsx}' -B 3 -A 10 'function.*Tab.*\{' # Look for manual prop in TabGroup usage context rg -g '*.{ts,tsx}' -B 2 -A 2 'TabGroup.*manual'Length of output: 147
packages/web/components/complex/portfolio/portfolio-page.tsx (2)
68-68: LGTM! Clean and simplified class names.The class names have been simplified by removing conditional styling, making the code more maintainable.
162-163: Verify empty state handling in OpenOrders component.The OpenOrders component is now rendered unconditionally, which aligns with the PR objective. However, let's verify that it properly handles empty states to ensure a good user experience.
✅ Verification successful
Based on the code inspection, I can now generate the final response:
Empty state handling is properly implemented in OpenOrders component
The component handles empty states by:
- Filtering orders to show only "open" status orders
- Checking for empty state via
hasOpenOrdersboolean- Returning
nullwhen there are no open orders (!hasOpenOrders) or while loading (isLoading)This is an appropriate implementation since:
- The component is used in a portfolio context where showing nothing when there are no orders is acceptable
- The parent component can handle the layout appropriately when this component returns null
- Loading states are properly handled to prevent flashing empty content
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if OpenOrders component handles empty states # Expected: Find empty state handling in the component # Search for empty state handling in OpenOrders component ast-grep --pattern 'const OpenOrders = $_ => { $$$ return ( $$$ {$_?.length === 0 && $_} $$$ ) }' # Alternative pattern to check for no data states rg -A 5 "no.*orders|empty.*orders|zero.*orders" --type tsxLength of output: 206
Script:
#!/bin/bash # Let's try different approaches to find the OpenOrders component and its empty state handling # Find the OpenOrders component file fd OpenOrders # Search for empty state patterns with proper file type rg -t ts -t tsx -A 5 "no.*orders|empty.*orders|zero.*orders" # Look for length checks or conditional rendering in files containing OpenOrders rg -t ts -t tsx "OpenOrders" -A 10 # Search for common empty state patterns rg -t ts -t tsx "length === 0|isEmpty|!.*length" -A 3 -B 1Length of output: 258
Script:
#!/bin/bash # Let's try without explicit file type flags since ripgrep seems to have issues with tsx extension # Find the OpenOrders component file fd OpenOrders # Look for the component implementation and empty states rg "OpenOrders" -A 10 # Search for common empty state patterns near OpenOrders mentions rg "length === 0|isEmpty|!.*length" --context=3 $(fd -t f OpenOrders) # Search for no data messages rg "no.*orders|empty.*orders|zero.*orders" $(fd -t f OpenOrders)Length of output: 463379
Script:
#!/bin/bash # Let's look for the OpenOrders component implementation to check empty state handling # Find the OpenOrders component file fd -t f "open-orders.tsx" # Look for empty state handling in the component rg -A 10 "const OpenOrders" $(fd -t f "open-orders.tsx") # Look for translations or text related to empty orders state rg "no.*orders|empty.*orders|zero.*orders" --type ts --type tsxLength of output: 618
Script:
#!/bin/bash # Let's try without file type flags since ripgrep seems to have issues with tsx extension # Look at the full implementation of OpenOrders component cat packages/web/components/complex/portfolio/open-orders.tsx # Also search for empty state handling patterns in the component rg "isLoading|hasOpenOrders|!orders|orders\." packages/web/components/complex/portfolio/open-orders.tsx # Look for any no orders related translations rg "portfolio.*orders" packages/web/localizations/Length of output: 5221
packages/web/components/transactions/transaction-details/transaction-details-content.tsx (2)
13-13: LGTM: Clean removal of unused feature flag import.The removal of
useFeatureFlagsaligns with the PR objective of removing limit orders flag dependencies.
84-86: LGTM: Simplified conditional styling.The styling logic has been streamlined to depend solely on the
isModalprop, making the behavior more predictable and maintainable.packages/web/components/pool-detail/concentrated.tsx (4)
157-157: LGTM!The container styling is appropriate for the layout requirements.
356-356: LGTM! Successfully removed feature flag dependency.The background styling is now consistently applied without conditional logic, aligning with the PR's objective to remove limit orders flag dependencies.
471-471: LGTM!The container styling appropriately structures the user assets section.
512-512: LGTM!The styling maintains consistency with other rounded card components in the UI.
packages/web/components/complex/orders-history/index.tsx (2)
223-223: LGTM: Style simplification aligns with PR objectiveThe removal of conditional rendering for the background class improves code maintainability while preserving the intended styling.
Line range hint
301-316: LGTM: Improved error handling for wallet interactionsThe enhanced error handling appropriately distinguishes between user-initiated request rejections and actual errors, providing better error tracking and user experience.
packages/web/components/complex/pools-table.tsx (1)
418-419: LGTM: Simplified table header stylingThe removal of conditional styling aligns with the PR objective to make styles permanent. The loading state animation is preserved while unnecessary conditional logic is removed.
packages/web/components/layouts/main.tsx (1)
51-51: LGTM! Style simplification aligns with PR objectives.The direct assignment of
xl:bg-osmoverse-1000class removes conditional styling logic, making the sidebar background permanent as intended.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
packages/web/styles/globals.css (1)
109-111: Remove commented code.The commented-out table header styles should be removed since they are no longer needed. This will help maintain a cleaner codebase and prevent confusion.
Apply this diff to remove the commented code:
- /* thead > tr { - @apply sticky top-navbar z-40 border-b border-osmoverse-700 bg-osmoverse-900 text-body2 font-semibold text-osmoverse-300 md:top-navbar-mobile; - } */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- packages/web/components/complex/pools-table.tsx (2 hunks)
- packages/web/components/table/asset-balances.tsx (1 hunks)
- packages/web/components/table/asset-info.tsx (1 hunks)
- packages/web/styles/globals.css (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/web/components/complex/pools-table.tsx
- packages/web/components/table/asset-balances.tsx
- packages/web/components/table/asset-info.tsx
🔇 Additional comments (1)
packages/web/styles/globals.css (1)
113-114: Verify the background color change across the application.The table header background color has been changed from
bg-osmoverse-900tobg-osmoverse-1000. Let's verify this change is consistent with other components that use table headers.✅ Verification successful
Background color change is consistent across the application
Based on the search results, we can confirm that:
- The global CSS file shows the background color was changed from
bg-osmoverse-900tobg-osmoverse-1000(visible in the commented out old code)- The component implementation in
packages/web/components/complex/orders-history/index.tsxuses the newbg-osmoverse-1000color- No other instances of table headers using the old
bg-osmoverse-900color were found- The styling is centralized in the global CSS file, which other components inherit from
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for any remaining instances of bg-osmoverse-900 in table headers # to ensure consistency with the new bg-osmoverse-1000 style. echo "Searching for potential inconsistencies in table header background colors..." rg -l "thead.*bg-osmoverse-900" --type css --type tsx --type jsx echo "Verifying the new background color is used consistently..." rg -l "thead.*bg-osmoverse-1000" --type css --type tsx --type jsxLength of output: 480
Script:
#!/bin/bash # Let's try a different approach by searching for any table header styling patterns # without file type restrictions since the previous attempt had issues with tsx extension echo "Searching for bg-osmoverse-900 in table headers..." rg "thead.*bg-osmoverse-900" -A 2 -B 2 echo "Searching for bg-osmoverse-1000 in table headers..." rg "thead.*bg-osmoverse-1000" -A 2 -B 2 echo "Searching for any thead styling patterns..." rg "thead.*bg-" -A 2 -B 2Length of output: 1453
Script:
#!/bin/bash # Let's search for table header styles in the global CSS and any other styling files # to get a complete picture of table header styling patterns echo "Searching in CSS files for thead styles..." fd -e css -x cat {} \; | rg "thead" -A 3 -B 3 echo "Searching for any other table styling patterns in components..." rg "table.*bg-osmoverse-[0-9]+" -A 2 -B 2Length of output: 1070
…-navigating-to-new-trade-page
mattupham
deleted the
mattupham/fe-1082-fix-background-flash-when-navigating-to-new-trade-page
branch
What is the purpose of the change:
Linear Task
https://linear.app/osmosis/issue/FE-1082/fix-background-flash-when-navigating-to-new-trade-page
Brief Changelog
Testing and Verifying
compare local changes (left) to prod (right) - ensure styles are the same