[do not merge] isTyping example with bridge quote state update #3035
Conversation
|
Important Review skippedIgnore keyword(s) in the title. Ignored keywords (2)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 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 Configuration File (
|
Deploying sanguine-fe with
|
| Latest commit: |
adb8a66
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://3525a36a.sanguine-fe.pages.dev |
| Branch Preview URL: | https://fe-is-typing-replica.sanguine-fe.pages.dev |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## fe/bridgeQuote-state #3035 +/- ##
==============================================================
+ Coverage 20.71326% 25.29346% +4.58020%
==============================================================
Files 490 781 +291
Lines 42509 56908 +14399
Branches 82 82
==============================================================
+ Hits 8805 14394 +5589
- Misses 32769 41027 +8258
- Partials 935 1487 +552
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Deploying sanguine with
|
| Latest commit: |
adb8a66
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://83a0b2fb.sanguine.pages.dev |
| Branch Preview URL: | https://fe-is-typing-replica.sanguine.pages.dev |
There was a problem hiding this comment.
PR Summary
This pull request introduces significant changes to the Synapse bridge functionality, focusing on improving state management, input handling, and bridge quote processing. Here are the key points:
-
Introduced a new
bridgeQuoteslice in Redux, separating quote-related state from general bridge state for better organization and performance. -
Implemented a new
isTypingstate and debounce mechanism in theInputContainercomponent to optimize UI updates during user input. -
Added new custom hooks:
useBridgeSelections,useBridgeValidations, anduseIsBridgeApprovedfor improved code organization and reusability. -
Refactored the bridge quote fetching logic into a separate Redux thunk, enhancing error handling and quote processing.
-
Removed the entire teaser page functionality, including components like
FauxBridge,Hero, andValueProps, suggesting a significant change in the application's structure or presentation.
26 file(s) reviewed, 7 comment(s)
Edit PR Review Bot Settings
| dataTestId="bridge-origin-token" | ||
| selectedItem={fromToken} | ||
| isOrigin={true} | ||
| placeholder="Out" |
There was a problem hiding this comment.
logic: placeholder 'Out' seems incorrect for origin token selector
| import { setFromToken } from '@/slices/bridge/reducer' | ||
| import { TokenSelector } from '@/components/ui/TokenSelector' | ||
| import { useBridgeState } from '@/slices/bridge/hooks' | ||
| import { useFromTokenListArray } from './hooks/useFromTokenListArray' | ||
| import { useWalletState } from '@/slices/wallet/hooks' |
There was a problem hiding this comment.
style: consider grouping imports by external and internal dependencies
| const isBridgeFeeGreaterThanInput = useMemo(() => { | ||
| return bridgeQuote.feeAmount === 0n && debouncedFromValueBigInt > 0n | ||
| }, [bridgeQuote.feeAmount, debouncedFromValueBigInt]) |
There was a problem hiding this comment.
logic: The logic for isBridgeFeeGreaterThanInput seems incorrect. It returns true when the fee is 0 and the input is greater than 0, which is the opposite of what the name suggests.
| const debouncedSetIsTyping = useCallback( | ||
| debounce((value: boolean) => setIsTyping?.(value), 1200), | ||
| [setIsTyping] | ||
| ) |
There was a problem hiding this comment.
style: Consider moving debouncedSetIsTyping outside the component to avoid recreating it on every render
| const handleInputChange = (event: React.ChangeEvent<HTMLInputElement>) => { | ||
| setIsTyping?.(true) | ||
| debouncedSetIsTyping(false) | ||
| handleFromValueChange?.(event) | ||
| } |
There was a problem hiding this comment.
logic: setIsTyping is called immediately with true, then debounced to false. This might lead to unnecessary state updates
|
|
||
| const handleInputChange = (event: React.ChangeEvent<HTMLInputElement>) => { | ||
| setIsTyping?.(true) | ||
| debouncedSetIsTyping(false) |
There was a problem hiding this comment.
style: debouncedSetIsTyping is always called with false. Consider passing the negation of the current typing state instead
| )) / | ||
| BigInt(originQuery.minAmountOut) | ||
|
|
||
| const isUnsupported = AcceptedChainId[fromChainId] ? false : true |
There was a problem hiding this comment.
style: This could be simplified to const isUnsupported = !AcceptedChainId[fromChainId]
Bundle ReportChanges will increase total bundle size by 6.15kB ⬆️
|
There was a problem hiding this comment.
PR Summary
(updates since last review)
This PR further refines the implementation of the 'isTyping' state in the bridge and swap components, building upon the changes introduced in the previous review. Key updates include:
- Increased debounce delay in InputContainer from 300ms to 400ms for updating 'fromValue', potentially improving performance but slightly delaying user feedback.
- Added 'isTyping' state to SwapInputContainer, including a debounced function for updating swap input value to reduce unnecessary state updates and API calls.
- Modified SwapTransactionButton to use 'isTyping' as a condition for disabling the button, preventing premature submissions during user input.
- Reduced AmountInput's setIsTyping debounce delay from 1200ms to 600ms, making the typing state update more responsive.
- Integrated 'isTyping' state into the StateManagedSwap component, passing it to relevant child components for improved user experience.
These changes aim to enhance the overall responsiveness and user experience of the bridge and swap functionalities while optimizing performance.
5 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings
| setShowValue(swapFromValueString) | ||
| debouncedUpdateSwapFromValue(swapFromValueString) |
There was a problem hiding this comment.
style: setShowValue is called before the debounced update, which may cause a slight UI flicker. Consider updating both simultaneously
null
296899f: synapse-interface preview link
29f7a33: synapse-interface preview link