Updates open short table #425
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| }); | ||
| }, | ||
| }), | ||
| columnHelper.accessor("hyperdriveAddress", { |
There was a problem hiding this comment.
This doesn't use the hyperdrive address, but an accessor string is required for the columnhelper.
There was a problem hiding this comment.
Curious where this comes from, can we not call it "currentValue"?
There was a problem hiding this comment.
Found the issue here, column helper has a display method that just renders whatever you want and you don't have to provide an accessor, which for us comes from OpenShort type.
| return ( | ||
| <> | ||
| {children({ showModal })} | ||
| {children && children({ showModal })} |
There was a problem hiding this comment.
total nit, but optional calling is nice here: {children?.()}
| header: `Size (hy${hyperdrive.baseToken.symbol})`, | ||
| cell: (bondAmount) => { | ||
| const bondAmountValue = bondAmount.getValue(); | ||
| return dnum.format([bondAmountValue, hyperdrive.baseToken.decimals], { |
There was a problem hiding this comment.
minor: We can use our formatBalance() utility for this, it's meant to help keep the dnum dependency contained.
| }); | ||
| }, | ||
| }), | ||
| columnHelper.accessor("hyperdriveAddress", { |
There was a problem hiding this comment.
Curious where this comes from, can we not call it "currentValue"?
| }); | ||
| const currentValue = | ||
| baseAmountOut && | ||
| dnum.format( |
There was a problem hiding this comment.
Same here, formatBalance
| return ( | ||
| <tr | ||
| key={row.id} | ||
| className="h-16 cursor-pointer grid-cols-4 items-center text-sm text-base-content even:bg-secondary/5 md:text-h6" |
There was a problem hiding this comment.
Let's add a hover as well, daisy-hover should do it
| const { address: account } = useAccount(); | ||
| const readHyperdrive = useReadHyperdrive(hyperdrive.address); | ||
| const queryEnabled = !!readHyperdrive && !!account; | ||
| const { data: shorts } = useQuery({ | ||
| queryKey: makeQueryKey("shortPositions", { account }), | ||
| queryFn: queryEnabled | ||
| ? () => readHyperdrive?.getOpenShorts({ account }) | ||
| : undefined, | ||
| enabled: queryEnabled, | ||
| }); |
There was a problem hiding this comment.
Can we avoid the parent having to fetch all this data and move this into the OpenShortTable intead?
| }: OpenOrdersTableProps): ReactElement { | ||
| const tableInstance = useReactTable({ | ||
| columns: columns(hyperdrive), | ||
| data: shorts, |
There was a problem hiding this comment.
Seems like if we move the data fetching into this commponent, it'd be okay to use [] while shorts is still fetching, wyt?
2 participants
This PR updates the open short table to use react-table. It implements 3 of the 5 columns that will ultimately be present for the trading competition. Those missing are hy-token price at open, and accrued yield. This also make the whole row clickable and removes the close position button in favor of the one on the modal.
