Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| const fieldStates = getTradeFormFieldStates(state, accountData, baseAccount); | ||
|
|
||
| const effectiveTrade = mapValues(fieldStates, (s) => s.effectiveValue) as TradeForm; | ||
| const effectiveTrade = mapValues(fieldStates, (s) => s?.effectiveValue) as TradeForm; |
There was a problem hiding this comment.
You shouldn't need the ?. This is because you are using ?: boolean instead of : boolean | undefined
| }, []); | ||
|
|
||
| return { | ||
| marketId: validId, |
There was a problem hiding this comment.
does it need to be returned? Can access from redux right?
| const { | ||
| type: selectedTradeType, | ||
| side = OrderSide.BUY, | ||
| // isClosingPosition, |
| // if (isClosingPosition === true) { | ||
| // return; | ||
| // } |
| tw="w-full" | ||
| /> | ||
|
|
||
| {useLimit && ( |
There was a problem hiding this comment.
This works but strays from how we consume the Trade form state.
const { showLimitPrice, showTriggerPrice } = tradeSummary.options;
This is why we have the makeVisible function
| {/* <DetachedSection> | ||
| <MobileBottomPanel /> | ||
| </DetachedSection> */} |
| size={ButtonSize.Medium} | ||
| onClick={() => { | ||
| navigate(`${AppRoute.TradeForm}`); | ||
| dispatch(tradeFormActions.setIsClosingPosition(false)); |
There was a problem hiding this comment.
I think always having to set this is actually lends support to us moving away from having it as a field type
| size={ButtonSize.Medium} | ||
| onClick={() => { | ||
| navigate(`${AppRoute.TradeForm}`); | ||
| dispatch(tradeFormActions.setIsClosingPosition(false)); |
| reduceOnly, | ||
| marginMode = MarginMode.CROSS, | ||
| side = OrderSide.BUY, | ||
| // isClosingPosition, |
| // if (isClosingPosition === true) { | ||
| // return; | ||
| // } |
| /> | ||
| </Button> | ||
| </div> | ||
| <div tw="flex w-full gap-0.25 rounded-[0.5em] bg-[#181819] p-[4px]"> |
There was a problem hiding this comment.
hardcoded bg color and px instead of rem
| </DropdownMenuTrigger> | ||
| </MobileDropdownMenu> | ||
| <AvailableRow> | ||
| <AvailableLabel> Available </AvailableLabel> |
There was a problem hiding this comment.
STRING_KEYS.AVAILABLE is available
| <TradeSizeInputs /> | ||
| </$InputsColumn> | ||
| <ToggleRow> | ||
| <ToggleLabel>Triggers</ToggleLabel> |
| const AvailableRow = styled.div` | ||
| display: flex; | ||
|
|
||
| justify-content: space-between; | ||
|
|
||
| align-items: center; | ||
|
|
||
| padding: 8 px 4 px; | ||
|
|
||
| margin-bottom: 8 px; | ||
|
|
||
| width: 100%; | ||
| `; | ||
| const AvailableLabel = styled.span` | ||
| color: #6b7280; | ||
|
|
||
| font-size: 15 px; | ||
| `; | ||
| const AvailableValue = styled.div` | ||
| display: flex; | ||
|
|
||
| align-items: center; | ||
|
|
||
| gap: 8 px; | ||
|
|
||
| color: #6b7280; | ||
|
|
||
| font-size: 15 px; | ||
| `; | ||
|
|
||
| const ToggleRow = styled.div` | ||
| display: flex; | ||
|
|
||
| justify-content: space-between; | ||
|
|
||
| align-items: center; | ||
|
|
||
| padding: 0px 0px; | ||
|
|
||
| width: 100%; | ||
| `; | ||
| const ToggleLabel = styled.span` | ||
| color: #9ca3af; | ||
|
|
||
| font-size: 16 px; | ||
|
|
||
| font-weight: 400; | ||
| `; |
There was a problem hiding this comment.
can we fix this to not use hardcoded colors and to use rem isntead of px
| content: <TvChart />, | ||
| forceMount: true, | ||
| label: stringGetter({ key: STRING_KEYS.PRICE }), | ||
| label: 'Chart', // stringGetter({ key: STRING_KEYS.CHART }), |
There was a problem hiding this comment.
add a TODO here. We can add Chart as soon as npm package ownership changes
| align="end" | ||
| tw="z-1" | ||
| items={sortItems} | ||
| slotTop={<span tw="text-color-text-0 font-small-book">Sort by</span>} |
| thead { | ||
| --stickyArea-totalInsetTop: 0; | ||
| } |
There was a problem hiding this comment.
is this something we want on both desktop and mobile?
| gap: 0.25em; | ||
| width: var(--output-width); | ||
|
|
||
| ${({ $isTablet }) => ($isTablet ? `` : 'width: var(--output-width); display: inline-flex;')} |
There was a problem hiding this comment.
can use a @media query instead of passing isTablet
| ${({ $isTablet }) => | ||
| `${$isTablet ? '--output-width: 30px; gap: 0.25em;' : '--output-width: 70px; gap: 0.75em; justify-content: center;'}`} | ||
| `; |
| </div> | ||
| <div tw="flex flex-[0.6] flex-col gap-0.125"> | ||
| <Button | ||
| tw="h-[34px] rounded-[8px] border-none bg-color-border-red px-1 text-mini text-red" |
| tw="h-[34px] rounded-[8px] border-none bg-color-border-red px-1 text-mini text-red" | ||
| onClick={() => onCloseButtonToggle(position.market)} | ||
| > | ||
| <Icon iconName={IconName.Close} size="12px" /> |
| onClick={() => onCloseButtonToggle(position.market)} | ||
| > | ||
| <Icon iconName={IconName.Close} size="12px" /> | ||
| Close |
| `; | ||
|
|
||
| const $Actions = styled.footer` | ||
| const $Actions = styled.div` |
There was a problem hiding this comment.
Don't quite understand this change.
I also don't think we use <PositionInfo> anywhere.
| {showMobileWeb ? <FooterMobile /> : <FooterDesktop />} | ||
|
|
||
| <NotificationsToastArea tw="z-[2] [grid-area:Main]" /> | ||
| <NotificationsToastArea tw="z-[2000] [grid-area:Main]" /> |
There was a problem hiding this comment.
this should be fine. but I think we need to have some reasoning or consistency behind assigning z-index to items.
2 participants
Changes
Issue
https://linear.app/dydx/issue/ENG-1185/revert-simple-ui-from-mobile-web
Related Issues (Optional)
Screenshots/Recordings (Optional)