(Deposit/Withdraw) Change bridge top bar on deposit address and remove awaiting BTC on more than 1 transfer #3987
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
4 Skipped Deployments
|
WalkthroughThe pull request introduces several changes across multiple files, primarily focusing on formatting consistency and component logic enhancements. In Changes
Possibly related PRs
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
packages/web/components/bridge/amount-screen.tsxOops! Something went wrong! :( ESLint: 8.50.0 ESLint couldn't find the config "next/core-web-vitals" to extend from. Please check that the name of the config is correct. The config "next/core-web-vitals" was referenced from the config file in "/packages/web/.eslintrc.json". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. 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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (17)
packages/web/localizations/de.jsonis excluded by!**/*.jsonpackages/web/localizations/en.jsonis excluded by!**/*.jsonpackages/web/localizations/es.jsonis excluded by!**/*.jsonpackages/web/localizations/fa.jsonis excluded by!**/*.jsonpackages/web/localizations/fr.jsonis excluded by!**/*.jsonpackages/web/localizations/gu.jsonis excluded by!**/*.jsonpackages/web/localizations/hi.jsonis excluded by!**/*.jsonpackages/web/localizations/ja.jsonis excluded by!**/*.jsonpackages/web/localizations/ko.jsonis excluded by!**/*.jsonpackages/web/localizations/pl.jsonis excluded by!**/*.jsonpackages/web/localizations/pt-br.jsonis excluded by!**/*.jsonpackages/web/localizations/ro.jsonis excluded by!**/*.jsonpackages/web/localizations/ru.jsonis excluded by!**/*.jsonpackages/web/localizations/tr.jsonis excluded by!**/*.jsonpackages/web/localizations/zh-cn.jsonis excluded by!**/*.jsonpackages/web/localizations/zh-hk.jsonis excluded by!**/*.jsonpackages/web/localizations/zh-tw.jsonis excluded by!**/*.json
📒 Files selected for processing (4)
packages/e2e/pages/trade-page.ts(1 hunks)packages/web/components/bridge/amount-screen.tsx(5 hunks)packages/web/components/nomic/nomic-pending-transfers.tsx(1 hunks)packages/web/hooks/bridge.tsx(5 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/e2e/pages/trade-page.ts
🔇 Additional comments (5)
packages/web/components/bridge/amount-screen.tsx (1)
612-615: Properly resetting type on component unmount
The use of useUnmount to reset the type in useBridgeStore to "quote" upon unmount is appropriate and ensures the store's state remains consistent when the component is unmounted.
packages/web/hooks/bridge.tsx (3)
39-39: Addition of type property to useBridgeStore
The introduction of the type state property with union types "quote" and "deposit-address" enhances the store's ability to manage different bridge modes effectively.
116-118: Implementation of setType method in useBridgeStore
The setType method is correctly implemented to update the type state within the store, providing a clear and maintainable way to manage the bridge type.
307-356: Conditional rendering of steps based on type
The logic to conditionally render the steps in the StepProgress component based on the type is well-designed. It ensures that the UI adapts correctly to the different bridge modes.
packages/web/components/nomic/nomic-pending-transfers.tsx (1)
168-172: Correct conditional rendering of awaiting BTC message
The updated logic correctly displays the awaiting BTC message only when there are no transactions, improving the user experience by preventing unnecessary messages when transactions are present.
| useBridgeStore.getState().setType("quote"); | ||
|
|
There was a problem hiding this comment.
Avoid mutating state during render
Similar to the previous issue, calling useBridgeStore.getState().setType("quote") directly within the render function can introduce side effects during rendering.
Move this state mutation into a useEffect hook to align with React best practices:
- useBridgeStore.getState().setType("quote");
/**
* This will trigger an error boundary
*/
if (!supportedSourceAssets && !isLoading) {
throw new Error("Supported source assets are not defined");
}
+ useEffect(() => {
+ useBridgeStore.getState().setType("quote");
+ }, []);Committable suggestion skipped: line range outside the PR's diff.
| @@ -802,6 +806,7 @@ | |||
| toChain && | |||
| toAsset | |||
| ) { | |||
| useBridgeStore.getState().setType("deposit-address"); | |||
There was a problem hiding this comment.
Avoid mutating state during render
Calling useBridgeStore.getState().setType("deposit-address") directly within the render path can cause side effects during rendering, which violates React's principle that render functions should be pure and side-effect free. This can lead to unexpected behavior and difficult-to-trace bugs.
Consider moving this state mutation into a useEffect hook to ensure it runs after the component renders:
- useBridgeStore.getState().setType("deposit-address");
return (
+ useEffect(() => {
+ useBridgeStore.getState().setType("deposit-address");
+ }, []);
+
<DepositAddressScreen
canonicalAsset={canonicalAsset}
direction={direction}
chainSelection={chainSelection}
assetDropdown={assetDropdown}
fromChain={fromChain}
toChain={toChain}
fromAsset={fromAsset}
toAsset={toAsset}
bridge={supportedBridgeInfo.depositAddressBridges[0]} // For now, only one bridge provider is supported
/>
);Committable suggestion skipped: line range outside the PR's diff.
JoseRFelix
deleted the
jose/fe-1266-change-bridging-top-bar-on-deposit-address-and-remove
branch
No description provided.