(Deposit/Withdraw) Allow Selecting a Custom Withdraw Address #3453
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Skipped Deployments
|
JoseRFelix
changed the title
…lecting-a-custom-withdraw-address
WalkthroughThe updates across multiple files primarily enhance the functionality of the Changes
Sequence Diagram(s)Manual Address Input and Transfer ConfirmationsequenceDiagram
participant User
participant AmountAndReviewScreen
participant BridgeTransferForm
participant BridgeNetworkSelectModal
participant BridgeWalletSelectModal
participant TransferService
User ->> AmountAndReviewScreen: Enter manualToAddress
AmountAndReviewScreen ->> BridgeTransferForm: Propagate manualToAddress
BridgeTransferForm ->> BridgeNetworkSelectModal: Open modal
BridgeNetworkSelectModal ->> User: Ask for manual address confirmation
User ->> BridgeNetworkSelectModal: Confirm manual address
BridgeNetworkSelectModal ->> AmountAndReviewScreen: Return confirmed address
AmountAndReviewScreen ->> BridgeWalletSelectModal: Open wallet select modal
BridgeWalletSelectModal ->> User: Show wallet options
User ->> BridgeWalletSelectModal: Select wallet
BridgeWalletSelectModal ->> AmountAndReviewScreen: Confirm wallet and address
AmountAndReviewScreen ->> TransferService: Initiate transfer
TransferService ->> User: Provide transfer status
Updated Chain Type and ID Logic Handling in SkipBridgeProvidersequenceDiagram
participant SkipBridgeProvider
participant ChainService
participant LogService
SkipBridgeProvider ->> ChainService: Get fromChain and toChain
ChainService -->> SkipBridgeProvider: Return chain info
SkipBridgeProvider ->> SkipBridgeProvider: Evaluate chain type and ID
alt if conditions met
SkipBridgeProvider ->> SkipBridgeProvider: Add addresses to addressList
LogService ->> SkipBridgeProvider: Log successful condition
else conditions not met
SkipBridgeProvider ->> SkipBridgeProvider: Skip address addition
LogService ->> SkipBridgeProvider: Log condition failure
end
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Files skipped from review as they are similar to previous changes (1)
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 as PR comments)
Additionally, you can add CodeRabbit Configration 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 (1)
packages/web/components/input/textarea-box.tsx (1)
93-93: Consider using optional chaining.The static analysis tool suggests using optional chaining for better readability and safety.
- onBlur && onBlur(e); + onBlur?.(e); - onFocus && onFocus(e); + onFocus?.(e);Also applies to: 97-97
Tools
Biome
[error] 93-93: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
Comment on lines
+84
to
+87
| const enum WalletSelectScreens { | ||
| WalletSelect = "wallet-select", | ||
| SendToAnotherAddress = "send-to-another-address", | ||
| } |
There was a problem hiding this comment.
Convert const enum to regular enum.
Const enums are not supported by bundlers and are incompatible with the 'isolatedModules' mode.
- const enum WalletSelectScreens {
+ enum WalletSelectScreens {Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const enum WalletSelectScreens { | |
| WalletSelect = "wallet-select", | |
| SendToAnotherAddress = "send-to-another-address", | |
| } | |
| enum WalletSelectScreens { | |
| WalletSelect = "wallet-select", | |
| SendToAnotherAddress = "send-to-another-address", | |
| } |
Tools
Biome
[error] 83-87: The enum declaration should not be const
Const enums are not supported by bundlers and are incompatible with the 'isolatedModules' mode. Their use can lead to import inexistent values.
See TypeScript Docs for more details.
Safe fix: Turn the const enum into a regular enum.(lint/suspicious/noConstEnum)
JoseRFelix
had a problem deploying
to
prod_swap_test
GitHub Actions
Failure
jonator
requested changes
Jul 8, 2024
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (2)
packages/web/components/bridge/immersive/amount-screen.tsx (2)
Line range hint
188-304:
Optimize the boolean check.Avoid redundant double-negation when a value will already be coerced to a boolean.
- if (!!account?.address) { + if (account?.address) {Tools
Biome
[error] 280-280: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation(lint/complexity/noExtraBooleanCast)
Line range hint
106-120:
Simplify nested conditional rendering.The nested conditional rendering can be simplified for better readability.
- if (readonly) { - return ( - <div className="subtitle1 flex w-[45%] flex-1 items-center gap-2 rounded-[48px] border border-osmoverse-700 py-2 px-4 text-osmoverse-200"> - <ChainLogo prettyName="" logoUri={chainLogo} color={chainColor} /> - <span className="truncate">{children}</span> - </div> - ); - } + return readonly ? ( + <div className="subtitle1 flex w-[45%] flex-1 items-center gap-2 rounded-[48px] border border-osmoverse-700 py-2 px-4 text-osmoverse-200"> + <ChainLogo prettyName="" logoUri={chainLogo} color={chainColor} /> + <span className="truncate">{children}</span> + </div> + ) : (
Comment on lines
+55
to
+112
| return ( | ||
| <div | ||
| className={classNames( | ||
| "flex h-fit w-full flex-nowrap justify-between rounded-lg bg-osmoverse-1000 px-2 text-white-high", | ||
| { | ||
| border: style !== "no-border", | ||
| "border-osmoverse-200": | ||
| style !== "no-border" && (style === "active" || textareaFocused), | ||
| "border-osmoverse-1000": | ||
| style !== "no-border" && style === "enabled" && !textareaFocused, | ||
| "border-missionError": style === "error", | ||
| "cursor-default border-white-disabled bg-osmoverse-800": disabled, | ||
| }, | ||
| className | ||
| )} | ||
| > | ||
| <div className={classNames("flex w-full shrink grow", classes?.label)}> | ||
| <textarea | ||
| id={id} | ||
| key={textareaKey} | ||
| ref={textareaRef} | ||
| className={classNames( | ||
| "md:leading-0 w-full resize-none appearance-none bg-transparent pt-px align-middle leading-10 outline-none placeholder:text-osmoverse-500 md:p-0", | ||
| { | ||
| "text-white-disabled": disabled, | ||
| "text-white-high": currentValue != "" && !disabled, | ||
| "float-right text-right": rightEntry, | ||
| "pr-1": !trailingSymbol, | ||
| }, | ||
| classes?.textarea | ||
| )} | ||
| value={textareaValue} | ||
| placeholder={placeholder ?? ""} | ||
| autoComplete="off" | ||
| onBlur={(e: any) => { | ||
| setTextareaFocused(false); | ||
| onBlur && onBlur(e); | ||
| }} | ||
| onFocus={(e: any) => { | ||
| setTextareaFocused(true); | ||
| onFocus && onFocus(e); | ||
| }} | ||
| onInput={(e: any) => setValue(e.target.value)} | ||
| disabled={disabled} | ||
| autoFocus={autoFocus} | ||
| rows={rows} | ||
| data-1p-ignore | ||
| data-enable-grammarly="false" | ||
| /> | ||
|
|
||
| {trailingSymbol && ( | ||
| <span className={classNames("pt-3", classes?.trailingSymbol)}> | ||
| {trailingSymbol} | ||
| </span> | ||
| )} | ||
| </div> | ||
| </div> | ||
| ); |
There was a problem hiding this comment.
Use optional chaining for safety and clarity.
Consider using optional chaining for onBlur and onFocus handlers to avoid potential runtime errors.
- onBlur={(e: any) => {
- setTextareaFocused(false);
- onBlur && onBlur(e);
- }}
+ onBlur={(e: any) => {
+ setTextareaFocused(false);
+ onBlur?.(e);
+ }}
- onFocus={(e: any) => {
- setTextareaFocused(true);
- onFocus && onFocus(e);
- }}
+ onFocus={(e: any) => {
+ setTextareaFocused(true);
+ onFocus?.(e);
+ }}Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| return ( | |
| <div | |
| className={classNames( | |
| "flex h-fit w-full flex-nowrap justify-between rounded-lg bg-osmoverse-1000 px-2 text-white-high", | |
| { | |
| border: style !== "no-border", | |
| "border-osmoverse-200": | |
| style !== "no-border" && (style === "active" || textareaFocused), | |
| "border-osmoverse-1000": | |
| style !== "no-border" && style === "enabled" && !textareaFocused, | |
| "border-missionError": style === "error", | |
| "cursor-default border-white-disabled bg-osmoverse-800": disabled, | |
| }, | |
| className | |
| )} | |
| > | |
| <div className={classNames("flex w-full shrink grow", classes?.label)}> | |
| <textarea | |
| id={id} | |
| key={textareaKey} | |
| ref={textareaRef} | |
| className={classNames( | |
| "md:leading-0 w-full resize-none appearance-none bg-transparent pt-px align-middle leading-10 outline-none placeholder:text-osmoverse-500 md:p-0", | |
| { | |
| "text-white-disabled": disabled, | |
| "text-white-high": currentValue != "" && !disabled, | |
| "float-right text-right": rightEntry, | |
| "pr-1": !trailingSymbol, | |
| }, | |
| classes?.textarea | |
| )} | |
| value={textareaValue} | |
| placeholder={placeholder ?? ""} | |
| autoComplete="off" | |
| onBlur={(e: any) => { | |
| setTextareaFocused(false); | |
| onBlur && onBlur(e); | |
| }} | |
| onFocus={(e: any) => { | |
| setTextareaFocused(true); | |
| onFocus && onFocus(e); | |
| }} | |
| onInput={(e: any) => setValue(e.target.value)} | |
| disabled={disabled} | |
| autoFocus={autoFocus} | |
| rows={rows} | |
| data-1p-ignore | |
| data-enable-grammarly="false" | |
| /> | |
| {trailingSymbol && ( | |
| <span className={classNames("pt-3", classes?.trailingSymbol)}> | |
| {trailingSymbol} | |
| </span> | |
| )} | |
| </div> | |
| </div> | |
| ); | |
| return ( | |
| <div | |
| className={classNames( | |
| "flex h-fit w-full flex-nowrap justify-between rounded-lg bg-osmoverse-1000 px-2 text-white-high", | |
| { | |
| border: style !== "no-border", | |
| "border-osmoverse-200": | |
| style !== "no-border" && (style === "active" || textareaFocused), | |
| "border-osmoverse-1000": | |
| style !== "no-border" && style === "enabled" && !textareaFocused, | |
| "border-missionError": style === "error", | |
| "cursor-default border-white-disabled bg-osmoverse-800": disabled, | |
| }, | |
| className | |
| )} | |
| > | |
| <div className={classNames("flex w-full shrink grow", classes?.label)}> | |
| <textarea | |
| id={id} | |
| key={textareaKey} | |
| ref={textareaRef} | |
| className={classNames( | |
| "md:leading-0 w-full resize-none appearance-none bg-transparent pt-px align-middle leading-10 outline-none placeholder:text-osmoverse-500 md:p-0", | |
| { | |
| "text-white-disabled": disabled, | |
| "text-white-high": currentValue != "" && !disabled, | |
| "float-right text-right": rightEntry, | |
| "pr-1": !trailingSymbol, | |
| }, | |
| classes?.textarea | |
| )} | |
| value={textareaValue} | |
| placeholder={placeholder ?? ""} | |
| autoComplete="off" | |
| onBlur={(e: any) => { | |
| setTextareaFocused(false); | |
| onBlur?.(e); | |
| }} | |
| onFocus={(e: any) => { | |
| setTextareaFocused(true); | |
| onFocus?.(e); | |
| }} | |
| onInput={(e: any) => setValue(e.target.value)} | |
| disabled={disabled} | |
| autoFocus={autoFocus} | |
| rows={rows} | |
| data-1p-ignore | |
| data-enable-grammarly="false" | |
| /> | |
| {trailingSymbol && ( | |
| <span className={classNames("pt-3", classes?.trailingSymbol)}> | |
| {trailingSymbol} | |
| </span> | |
| )} | |
| </div> | |
| </div> | |
| ); |
Tools
Biome
[error] 91-91: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 95-95: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
jonator
approved these changes
Jul 8, 2024
JoseRFelix
deleted the
jose/fe-619-allow-selecting-a-custom-withdraw-address
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR introduces the ability to add a custom address for withdrawals. It also makes wallet connection and switching entirely predictable and imperative by not relying on
useEffects.Linear Task
Linear Task URL