Skip to content
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

feat: mobile view of route book #124

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from
Draft

Conversation

vacekj
Copy link
Member

@vacekj vacekj commented Nov 4, 2024

fixes #103

@vacekj vacekj linked an issue Nov 4, 2024 that may be closed by this pull request
@vacekj vacekj force-pushed the 103-mobile-view-of-route-book branch 4 times, most recently from 236b354 to 3ad0a68 Compare November 6, 2024 21:50
@vacekj vacekj requested review from JasonMHasperhoven and grod220 and removed request for JasonMHasperhoven November 6, 2024 21:55
Comment on lines +21 to +30
<div className='flex items-center gap-1 py-2 text-xs text-white'>
{tokens.map((token, index) => (
<React.Fragment key={token}>
{index > 0 && <ChevronRight className='w-3 h-3 text-gray-400' />}
<span>{token}</span>
</React.Fragment>
))}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue:

  1. It should display on hover. At the moment it is on-click.
  2. When it is displaying, it seems to increase in height ever so slightly.

src/pages/trade/ui/route-book.tsx Outdated Show resolved Hide resolved
src/pages/trade/ui/route-book.tsx Outdated Show resolved Hide resolved
src/pages/trade/ui/route-book.tsx Outdated Show resolved Hide resolved
<td className='text-right tabular-nums'>{trace.hops.length}</td>
<div className='flex flex-col max-w-full border-y border-[#262626]'>
<div className='flex items-center gap-2 px-4 h-11 border-b border-[#262626]'>
<Tabs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: I don't suppose there is a prop or something for condensed is there? It feels a bit too big right?

src/pages/trade/ui/route-book.tsx Outdated Show resolved Hide resolved

return (
<tr>
<td colSpan={4} className='border-y border-[#262626]'>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: #262626 (and all the other color strings) should probably be a constant too right? It's likely we should have these constants in some global place somewhere. Maybe in the tailwind config (?) until we have the UI library updated I suppose.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch - tailwind config sounds good to me for now, then directly in the UI pkg later.

src/pages/trade/ui/route-book.tsx Outdated Show resolved Hide resolved
src/pages/trade/ui/route-book.tsx Outdated Show resolved Hide resolved
Comment on lines 139 to 151
if (bookIsLoading || !bookData) {
return <div className='text-gray-400'>Loading...</div>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue: if there is an error, the !bookData will guarantee it will be stuck in this loading state. Think the previous ordering prevented that possibility.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: Perhaps the loading skeleton can be included too? Or if not in this PR, perhaps a subsequent PR?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch - will include the loading skeleton in this PR.

@vacekj vacekj force-pushed the 103-mobile-view-of-route-book branch from 4fb2bb3 to bd65241 Compare November 9, 2024 22:13
feat: correct table

feat: display hop tokens correctly
• Removed unused tokens array
• Removed unused Route Depth tabˆ
• Removed single hop data and only use multi hop data
• Removed liquidity percentage and isDirect props from TradeRow component
• Removed SpreadRow component and replaced it with a new implementation that calculates the spread between sell and buy orders
• Removed unused code and simplified the RouteBookData component
• Added a new calculateSpread function to calculate the spread between sell and buy orders
• Un-commented Summary component
• Replaced custom TabButton with Tabs from @penumbra-zone/ui
• Removed console.log statement in TradeRow component
• Updated table headers in RouteBookData component to correct symbol order
• Removed unused width from RouteBookData table cell
• Add relative size calculation for trade rows
• Display relative size as a background gradient in trade rows
• Pass relative size as a prop to TradeRow component
• Calculate relative sizes for sell and buy orders separately
@vacekj vacekj force-pushed the 103-mobile-view-of-route-book branch from bd65241 to 40c92cb Compare November 13, 2024 23:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mobile view of Route book
2 participants