-
Notifications
You must be signed in to change notification settings - Fork 415
FEDEV-57: Tradebox UX upgrade #2073
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: release
Are you sure you want to change the base?
Conversation
Deploying gmx-interface-home with
|
| Latest commit: |
b128ae5
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://51619a44.gmx-interface-home.pages.dev |
| Branch Preview URL: | https://tradebox-ux-update.gmx-interface-home.pages.dev |
Deploying gmx-interface with
|
| Latest commit: |
b128ae5
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://106ec1ef.gmx-interface.pages.dev |
| Branch Preview URL: | https://tradebox-ux-update.gmx-interface.pages.dev |
|
@cursor review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| )} | ||
|
|
||
| {networkFee && ( | ||
| {networkFee && isTpSlEnabled && ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Network fee hidden when editing order without TP/SL
High Severity
The network fee row condition networkFee && isTpSlEnabled prevents displaying execution fees when editing an order without adding TP/SL orders. The additionalExecutionFee (shown in the tooltip as "Order update fee") exists independently when gas prices have increased since order creation. Users editing orders won't see the fees they're about to pay unless they also enable TP/SL.
| setIsTpSlEnabled(false); | ||
| setTpPriceInputValue(""); | ||
| setSlPriceInputValue(""); | ||
| }, [p.order.key, setIsTpSlEnabled, setSlPriceInputValue, setTpPriceInputValue]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate effect resets TP/SL state twice on order change
Low Severity
Two effects reset the same TP/SL state (setIsTpSlEnabled, setTpPriceInputValue, setSlPriceInputValue) when the order key changes. One is in useOrderEditorTPSL hook and another in OrderEditor.tsx, both calling the same setters from the hook. This duplication causes unnecessary double state resets and makes the code harder to maintain.
Additional Locations (1)
| const { fees, executionFee } = useSelector(selectPositionSellerFees); | ||
|
|
||
| const triggerPrice = useSelector(selectPositionSellerTriggerPrice); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TWAP acceptable price impact always shows NA after trigger removal
Medium Severity
The AcceptablePriceImpactInputRow condition notAvailable={!triggerPriceInputValue || isStopLoss || !decreaseAmounts} causes the row to always display "NA" for TWAP orders. Since the trigger price input UI was removed from PositionSeller, triggerPriceInputValue is always empty, making !triggerPriceInputValue always true. Users with isSetAcceptablePriceImpactEnabled enabled cannot customize acceptable price impact for TWAP orders even though the UI attempts to show this option.
Main changes:
Updated Tradebox:
Other changes: