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.

Sign up for GitHub

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

Jump to bottom

chore: implement fix to limit amount of accounts rendered #5170

Merged
merged 1 commit into from
Apr 5, 2024
Merged

chore: implement fix to limit amount of accounts rendered #5170

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion src/app/features/dialogs/switch-account-dialog/switch-account-dialog.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export const SwitchAccountDialog = memo(({ isShowing, onClose }: DialogProps) =>
if (!isShowing) return null;

const accountNum = stacksAddressesNum || btcAddressesNum;
const maxAccountsShown = accountNum > 10 ? 10 : accountNum;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kyranjamie - you were right about this and I had naively set the list to have a generated height based on all of the accounts. This change limits it to only allow the height of 10 accounts to be rendered.

When I test I still see more than 10 rendered so I need to figure that out. I see 15 items rendered when I can only see 9 in the viewport.

The reason I changed this code was so that the UI could smoothly adjust and increase the list height on smaller width screens. Adding this code impacts that but I am close to having a better fix in: #5171

I just thought it best to get something mergeable for this as it was a bad bug on dev and a release blocker


return (
<Dialog
Expand All @@ -64,7 +65,7 @@ export const SwitchAccountDialog = memo(({ isShowing, onClose }: DialogProps) =>
height={virtuosoHeight}
style={{
...virtuosoStyles,
height: `calc(${virtuosoHeight * accountNum}px + ${getHeightOffset(true, !isLedger)}px)`,
height: `calc(${virtuosoHeight * maxAccountsShown}px + ${getHeightOffset(true, !isLedger)}px)`,
}}
initialTopMostItemIndex={whenWallet({ ledger: 0, software: currentAccountIndex })}
totalCount={stacksAddressesNum || btcAddressesNum}
Expand Down
4 changes: 3 additions & 1 deletion src/app/pages/choose-account/components/accounts.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,14 +112,16 @@ export const ChooseAccountsList = memo(() => {
if (!accounts) return null;
const accountNum = accounts.length;

const maxAccountsShown = accountNum > 10 ? 10 : accountNum;

return (
<Box mt="space.05" mb="space.06" width="100%">
{whenWallet({ software: <AddAccountAction />, ledger: <></> })}
<Virtuoso
height={virtuosoHeight}
style={{
...virtuosoStyles,
height: `calc(${virtuosoHeight * accountNum}px + 50px)`,
height: `calc(${virtuosoHeight * maxAccountsShown}px + 50px)`,
background: token('colors.ink.background-primary'),
}}
data={accounts}
Expand Down
3 changes: 2 additions & 1 deletion ...send-crypto-asset-form/components/recipient-accounts-dialog/recipient-accounts-dialog.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,15 @@ export function RecipientAccountsDialog() {

if (stacksAddressesNum === 0 && btcAddressesNum === 0) return null;
const accountNum = stacksAddressesNum || btcAddressesNum;
const maxAccountsShown = accountNum > 10 ? 10 : accountNum;

return (
<Dialog header={<Header variant="dialog" title="My accounts" />} isShowing onClose={onGoBack}>
<Virtuoso
height={virtuosoHeight}
style={{
...virtuosoStyles,
height: `calc(${virtuosoHeight * accountNum}px + ${getHeightOffset(true, true)}px)`,
height: `calc(${virtuosoHeight * maxAccountsShown}px + ${getHeightOffset(true, true)}px)`,
}}
itemContent={index => (
<Box key={index} my="space.05" px="space.05">
Expand Down
Loading