views: improve approval dialogue rendering #241
Conversation
packages/ui/components/ui/tx/actions-views/swap/one-way-swap.tsx
Outdated
Show resolved
Hide resolved
|
Looks great visually! Agree on leaving the rest of the improvements for later once the V2 designs are complete |
packages/ui/components/ui/tx/actions-views/swap/one-way-swap.tsx
Outdated
Show resolved
Hide resolved
| <span className='truncate font-mono text-xs text-muted-foreground max-w-[80px]'> | ||
| {symbol} | ||
| </span> |
There was a problem hiding this comment.
truncates the value component for staking, delegating, unbonding, etc.
before
after
| className='flex flex-row justify-between gap-4 rounded-md p-4 shadow-md' | ||
| style={{ | ||
| backgroundColor: '#1A1A1A', | ||
| paddingBottom: '28px', | ||
| paddingTop: '28px', | ||
| }} |
There was a problem hiding this comment.
blending the colors of the layer containing the approve and deny buttons
before
after
| className='w-1/2 py-3.5 text-base hover:bg-destructive/90' | ||
| size='lg' |
There was a problem hiding this comment.
fix dimensions of site settings button
before
after
| <ValueWithAddress addressView={address} label='from'> | ||
| <></> | ||
| </ValueWithAddress> |
There was a problem hiding this comment.
question: This is the same as
<ValueWithAddress addressView={address} label='from' />But is that intended?
There was a problem hiding this comment.
I changed it since It was throwing an error related to missing attributes, but functionally it's the same.
packages/ui/components/ui/tx/actions-views/swap/one-way-swap.tsx
Outdated
Show resolved
Hide resolved
| <div className='flex items-center justify-between gap-3'> | ||
| <ValueViewComponent view={note.value} /> | ||
| </ValueWithAddress> | ||
| <ValueWithAddress addressView={address} label='to'> | ||
| <></> | ||
| </ValueWithAddress> | ||
| </div> |
There was a problem hiding this comment.
issue: a <></> is basically empty. Also, think addresses are getting cut off when there should be a line break likely.
There was a problem hiding this comment.
Good catch! This was refactored to place everything on the same line, but it unintentionally cuts off transfers to external wallet addresses. I truncated the ValueWithAddress component with a max width to fix this.
| )} | ||
| </span> | ||
| </div> | ||
| {visibleContent} | ||
| {visibleContent && <div className='border-t border-gray-700 pt-2'>{visibleContent}</div>} |
There was a problem hiding this comment.
suggestion: personally, I think you can get rid of the horizontal line border
There was a problem hiding this comment.
I actually prefer the horizontal line border because it provides clearer visual separation and enhances the structure.
|
addressed feeback, and any additional design work can be handled in a separate PR. |
references #240
I opted not to import the new
valueViewComponentsince the UI V2 library is still in-flight and not finalized. To avoid adding further complexity — especially given the existing lack of uniformity across UI libraries (deprecated V1, V2, and the prax-specific UI) — I reused the existingvalueViewComponent.According to the developer docs, the maximum size for an extension popup is limited to 800x600 unfortunately.
Moreover, I took some design liberties in the approval dialogue by refactoring the css and tightening the dimensions, but this probably goes beyond the scope of what was intended.
before
Screen.Recording.2024-11-22.at.8.19.05.AM.mov
after
Screen.Recording.2024-11-22.at.8.10.03.AM.mov