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

views: improve approval dialogue rendering #241

Merged
merged 8 commits into from
Nov 25, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 18 additions & 5 deletions apps/extension/src/routes/popup/approval/approve-deny.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,25 @@ export const ApproveDeny = ({
const count = useWindowCountdown(wait);

return (
<div className='flex flex-row flex-wrap justify-center gap-4 bg-black p-4 shadow-lg'>
<Button variant='gradient' className='w-full' size='lg' onClick={approve} disabled={!!count}>
<div
className='flex flex-row justify-between gap-4 rounded-md p-4 shadow-md'
style={{
backgroundColor: '#1A1A1A',
paddingBottom: '28px',
paddingTop: '28px',
}}
Comment on lines +19 to +24
Copy link
Contributor Author

Choose a reason for hiding this comment

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

blending the colors of the layer containing the approve and deny buttons


before

Screenshot 2024-11-24 at 9 45 58 PM Screenshot 2024-11-24 at 9 47 19 PM

after

Screenshot 2024-11-24 at 9 42 50 PM Screenshot 2024-11-24 at 9 42 38 PM

TalDerei marked this conversation as resolved.
Show resolved Hide resolved
>
<Button
variant='gradient'
className='w-1/2 py-3.5 text-base'
size='lg'
onClick={approve}
disabled={!!count}
>
Approve {count !== 0 && `(${count})`}
</Button>
<Button
className='min-w-[50%] grow items-center gap-2 hover:bg-destructive/90'
className='w-1/2 py-3.5 text-base hover:bg-destructive/90'
size='lg'
variant='destructiveSecondary'
onClick={deny}
Expand All @@ -29,8 +42,8 @@ export const ApproveDeny = ({
</Button>
{ignore && (
<Button
className='w-1/3 hover:bg-destructive/90'
size='lg'
className='w-[32%] py-2.5 text-sm hover:bg-destructive/90'
size='md'
variant='secondary'
onClick={ignore}
>
Expand Down
23 changes: 16 additions & 7 deletions apps/extension/src/routes/popup/approval/transaction/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,15 @@ export const TransactionApproval = () => {

return (
<div className='flex h-screen flex-col'>
<div className='grow overflow-auto p-[30px] pt-10'>
<p className='bg-text-linear bg-clip-text pb-2 font-headline text-2xl font-bold text-transparent'>
Confirm transaction
</p>

<div className='border-b border-gray-700 p-4'>
<h1
className=' bg-text-linear bg-clip-text pb-2 font-headline text-2xl font-bold text-transparent'
style={{ paddingBottom: '0px' }}
TalDerei marked this conversation as resolved.
Show resolved Hide resolved
>
Confirm Transaction
</h1>
</div>
<div className='grow overflow-auto p-4'>
<ViewTabs
defaultValue={selectedTransactionViewName}
onValueChange={setSelectedTransactionViewName}
Expand All @@ -60,13 +64,18 @@ export const TransactionApproval = () => {
<TransactionViewComponent txv={selectedTransactionView} metadataFetcher={getMetadata} />

{selectedTransactionViewName === TransactionViewTab.SENDER && (
<div className='mt-8'>
<div className='mt-4'>
<JsonViewer jsonObj={authorizeRequest.toJson() as Jsonified<AuthorizeRequest>} />
</div>
)}
</div>

<ApproveDeny approve={approve} deny={deny} wait={3} />
<div
className='border-t border-gray-700 p-4'
style={{ paddingBottom: '0px', paddingTop: '0px', paddingRight: '0px', paddingLeft: '0px' }}
TalDerei marked this conversation as resolved.
Show resolved Hide resolved
>
<ApproveDeny approve={approve} deny={deny} wait={3} />
</div>
</div>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export const ViewTabs = ({
onValueChange={value => onValueChange(value as TransactionViewTab)}
>
<TabsList
className={cn('mx-auto mb-8 grid w-[100%] gap-4', {
className={cn('mx-auto mb-12 grid w-[100%] gap-4', {
'grid-cols-2': !showReceiverTransactionView,
'grid-cols-3': showReceiverTransactionView,
})}
Expand Down
6 changes: 3 additions & 3 deletions apps/extension/src/routes/popup/home/index.tsx
TalDerei marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -50,16 +50,16 @@ export const PopupIndex = () => {
<>
<BlockSync />

<div className='flex h-full grow flex-col items-stretch gap-[30px] bg-logo bg-left-bottom px-[30px] pb-[30px]'>
<div className='flex h-full grow flex-col items-stretch gap-[15px] bg-logo bg-left-bottom px-[15px] pb-[15px]'>
<IndexHeader />

<div className='flex flex-col gap-8'>
<div className='flex flex-col gap-4'>
{activeWallet && <SelectAccount getAddrByIndex={getAddrByIndex(activeWallet)} />}
</div>

<ValidateAddress />

<div className='grow' />
<div className='shrink-0 grow' />

<FrontendLink />
</div>
Expand Down
1 change: 1 addition & 0 deletions packages/ui/components/ui/tabs/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const TabsList = React.forwardRef<
'inline-flex h-[52px] items-center justify-center rounded-lg bg-background px-2',
className,
)}
style={{ marginBottom: '16px' }}
TalDerei marked this conversation as resolved.
Show resolved Hide resolved
{...props}
/>
));
Expand Down
7 changes: 5 additions & 2 deletions packages/ui/components/ui/tx/actions-views/output.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,12 @@ export const OutputViewComponent = ({ value }: { value: OutputView }) => {
<ViewBox
label='Output'
visibleContent={
<ValueWithAddress addressView={address} label='to'>
<div className='flex items-center justify-between gap-3'>
<ValueViewComponent view={note.value} />
</ValueWithAddress>
<ValueWithAddress addressView={address} label='to'>
<></>
</ValueWithAddress>
</div>
Comment on lines +17 to +22
Copy link
Contributor

Choose a reason for hiding this comment

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

issue: a <></> is basically empty. Also, think addresses are getting cut off when there should be a line break likely.

Screenshot 2024-11-25 at 10 58 44 AM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Screenshot 2024-11-25 at 8 32 43 AM Screenshot 2024-11-25 at 8 32 58 AM

}
/>
);
Expand Down
9 changes: 6 additions & 3 deletions packages/ui/components/ui/tx/actions-views/spend.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { SpendView } from '@penumbra-zone/protobuf/penumbra/core/component/shielded_pool/v1/shielded_pool_pb';
import { ViewBox } from '../viewbox';
import { ValueViewComponent } from '../../value';
import { ValueWithAddress } from './value-with-address';
import { getNote } from '@penumbra-zone/getters/spend-view';
import { getAddress } from '@penumbra-zone/getters/note-view';
import { ValueViewComponent } from '../../value';

export const SpendViewComponent = ({ value }: { value: SpendView }) => {
if (value.spendView.case === 'visible') {
Expand All @@ -14,9 +14,12 @@ export const SpendViewComponent = ({ value }: { value: SpendView }) => {
<ViewBox
label='Spend'
visibleContent={
<ValueWithAddress addressView={address} label='from'>
<div className='flex items-center justify-between gap-3'>
<ValueViewComponent view={note.value} />
</ValueWithAddress>
<ValueWithAddress addressView={address} label='from'>
<></>
</ValueWithAddress>
Comment on lines +19 to +21
Copy link
Contributor

Choose a reason for hiding this comment

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

question: This is the same as

  <ValueWithAddress addressView={address} label='from' />

But is that intended?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed it since It was throwing an error related to missing attributes, but functionally it's the same.

Screenshot 2024-11-25 at 8 36 51 AM

</div>
}
/>
);
Expand Down
37 changes: 31 additions & 6 deletions packages/ui/components/ui/tx/actions-views/swap/one-way-swap.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { ValueViewComponent } from '../../../value';
import { ArrowRight } from 'lucide-react';
import { ValueView } from '@penumbra-zone/protobuf/penumbra/core/asset/v1/asset_pb';
import { getAmount } from '@penumbra-zone/getters/value-view';

Expand All @@ -13,12 +12,38 @@
const outputAmount = getAmount.optional(output);

return (
<div className='flex items-center gap-2'>
<div className='flex items-center justify-between'>
<ValueViewComponent view={input} />

<ArrowRight />

<ValueViewComponent view={output} showValue={!!outputAmount} />
<div className='relative mx-2 flex items-center justify-center'>
<div className='flex items-center justify-center w-16 h-6 rounded-full bg-gray-800 border border-gray-500'>

Check warning on line 18 in packages/ui/components/ui/tx/actions-views/swap/one-way-swap.tsx

View workflow job for this annotation

GitHub Actions / Lint

Invalid Tailwind CSS classnames order
TalDerei marked this conversation as resolved.
Show resolved Hide resolved
TalDerei marked this conversation as resolved.
Show resolved Hide resolved
<div
className='flex items-center justify-center'
style={{
position: 'relative',
}}
TalDerei marked this conversation as resolved.
Show resolved Hide resolved
>
<div
className='flex items-center'
style={{
width: '14px',
height: '2px',
backgroundColor: 'white',
}}
TalDerei marked this conversation as resolved.
Show resolved Hide resolved
/>
<div
className='w-0 h-0 ml-[2px]'

Check warning on line 34 in packages/ui/components/ui/tx/actions-views/swap/one-way-swap.tsx

View workflow job for this annotation

GitHub Actions / Lint

Invalid Tailwind CSS classnames order

Check warning on line 34 in packages/ui/components/ui/tx/actions-views/swap/one-way-swap.tsx

View workflow job for this annotation

GitHub Actions / Lint

Classnames 'w-0, h-0' could be replaced by the 'size-0' shorthand!
style={{
borderTop: '4px solid transparent',
borderBottom: '4px solid transparent',
borderLeft: '6px solid white',
}}
TalDerei marked this conversation as resolved.
Show resolved Hide resolved
/>
</div>
</div>
</div>
<div className='flex items-center justify-end'>
<ValueViewComponent view={output} showValue={!!outputAmount} />
</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: swap-claim fee seems to have a different font

Screenshot 2024-11-25 at 10 53 48 AM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

modified to text-base – suggesting we conduct an audit of the typography styles being used and and cross-reference with the figma designs per penumbra-zone/web#1921.

</div>
);
};
6 changes: 3 additions & 3 deletions packages/ui/components/ui/tx/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,14 @@ export const TransactionViewComponent = ({
const { feeValueView, isLoading, error } = useFeeMetadata(txv, metadataFetcher);

return (
<div className='flex flex-col gap-8'>
<div className='flex flex-col gap-4'>
{txv.bodyView?.memoView?.memoView && <MemoViewComponent memo={txv.bodyView.memoView} />}
<ViewSection heading='Actions'>
<ViewSection heading={<div style={{ paddingLeft: '3px' }}>Actions</div>}>
TalDerei marked this conversation as resolved.
Show resolved Hide resolved
{txv.bodyView?.actionViews.map((av, i) => (
<ActionViewComponent av={av} feeValueView={feeValueView} key={i} />
))}
</ViewSection>
<ViewSection heading='Parameters'>
<ViewSection heading={<div style={{ paddingLeft: '3px' }}>Parameters</div>}>
TalDerei marked this conversation as resolved.
Show resolved Hide resolved
<ViewBox
label='Transaction Fee'
visibleContent={
Expand Down
4 changes: 2 additions & 2 deletions packages/ui/components/ui/tx/memo-view.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@ export const MemoViewComponent = ({ memo: { memoView } }: { memo: MemoView }) =>
<ViewBox
label='Memo'
visibleContent={
<div className='flex flex-col gap-4'>
<div className='flex flex-col gap-2'>
<ActionDetails>
<ActionDetails.Row label='Return Address'>
<AddressViewComponent view={memoView.value.plaintext!.returnAddress} />
</ActionDetails.Row>
<ActionDetails.Row label='Memo Text'>
<span className='italic' style={{ wordBreak: 'normal' }}>
<span className='text-sm italic' style={{ wordBreak: 'normal' }}>
{memoView.value.plaintext?.text}
</span>
</ActionDetails.Row>
Expand Down
31 changes: 15 additions & 16 deletions packages/ui/components/ui/tx/viewbox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,44 +11,43 @@ export interface ViewBoxProps {
isOpaque?: boolean;
}

const Label = ({ label }: { label: string }) => <span className='text-lg'>{label}</span>;
const Label = ({ label }: { label: string }) => (
<span className='text-lg font-bold text-gray-300'>{label}</span>
);

export const ViewBox = ({ label, visibleContent, isOpaque }: ViewBoxProps) => {
// if isOpaque is undefined, set it to !visibleContent
isOpaque = isOpaque ?? !visibleContent;
isOpaque = isOpaque ?? !visibleContent?.props;

return (
<Box overflow='hidden'>
<div
className={cn(
'flex flex-col gap-1 break-all overflow-hidden',
isOpaque ? 'cursor-not-allowed' : '',
)}
>
<div className='flex items-center gap-2 self-start'>
<span className={cn('text-base font-bold', isOpaque ? 'text-gray-600' : '')}>
{!isOpaque && <Label label={label} />}
{isOpaque && (
<div className='flex gap-2'>
<div className={cn('flex flex-col gap-2', isOpaque ? 'cursor-not-allowed' : '')}>
<div className='flex items-center gap-2'>
<span className={cn('text-base', isOpaque ? 'text-gray-600' : '')}>
{isOpaque ? (
<div className='flex items-center gap-2'>
<IncognitoIcon fill='#4b5563' />
<Label label={label} />
</div>
) : (
<Label label={label} />
)}
</span>
</div>
{visibleContent}
{visibleContent && <div className='border-t border-gray-700 pt-2'>{visibleContent}</div>}
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: personally, I think you can get rid of the horizontal line border

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually prefer the horizontal line border because it provides clearer visual separation and enhances the structure.

Screenshot 2024-11-25 at 1 33 02 PM Screenshot 2024-11-25 at 1 32 24 PM

</div>
</Box>
);
};

export interface ViewSectionProps {
heading: string;
heading: React.ReactNode;
children?: React.ReactNode;
}

export const ViewSection = ({ heading, children }: ViewSectionProps) => {
return (
<div className='grid gap-4'>
<div className='grid gap-2'>
<div className='text-xl font-bold'>{heading}</div>
{children}
</div>
Expand Down
4 changes: 3 additions & 1 deletion packages/ui/components/ui/value/value.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@
</span>
)}
{showDenom && (
<span className='truncate font-mono text-xs text-muted-foreground'>{symbol}</span>
<span className='truncate font-mono text-xs text-muted-foreground max-w-[80px]'>

Check warning on line 41 in packages/ui/components/ui/value/value.tsx

View workflow job for this annotation

GitHub Actions / Lint

Invalid Tailwind CSS classnames order
{symbol}
</span>
Copy link
Contributor Author

@TalDerei TalDerei Nov 25, 2024

Choose a reason for hiding this comment

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

truncates the value component for staking, delegating, unbonding, etc.


before

Confirm Transaction

after

Screenshot 2024-11-24 at 9 38 49 PM Screenshot 2024-11-24 at 9 39 21 PM

)}
</div>
</Pill>
Expand Down