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

fix: allow virtuoso to resize dynamically, ref #5242 #5289

Merged
merged 1 commit into from
Apr 24, 2024
Merged

fix: allow virtuoso to resize dynamically, ref #5242 #5289

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
23 changes: 23 additions & 0 deletions src/app/common/hooks/use-media-query.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { useEffect, useState } from 'react';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Re-adding this which I naively removed as I thought we didn't need it, however it's important as it alters the breakpoint on re-size


import { BreakpointToken, token } from 'leather-styles/tokens';

function useMediaQuery(query: string) {
const [matches, setMatches] = useState(false);

useEffect(() => {
const media = window.matchMedia(query);
if (media.matches !== matches) {
setMatches(media.matches);
}
pete-watters marked this conversation as resolved.
Show resolved Hide resolved
const listener = () => setMatches(media.matches);
window.addEventListener('resize', listener);
return () => window.removeEventListener('resize', listener);
}, [matches, query]);

return matches;
}

export function useViewportMinWidth(viewport: BreakpointToken) {
return useMediaQuery(`(min-width: ${token(`breakpoints.${viewport}`)})`);
}
12 changes: 7 additions & 5 deletions src/app/features/container/container.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,10 +97,12 @@ export function Container() {
const isLogoClickable = variant !== 'home' && !isRpcRoute(pathname);
return (
<>
<SwitchAccountDialog
isShowing={isShowingSwitchAccount}
onClose={() => setIsShowingSwitchAccount(false)}
/>
{isShowingSwitchAccount && (
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There was a mistake here in that <SwitchAccountDialog shouldn't be in the DOM unless showing

Copy link
Contributor

Choose a reason for hiding this comment

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

nice catch! I believe problem with requests not being cancelled on close in SwitchAccountDialog is here

<SwitchAccountDialog
isShowing={isShowingSwitchAccount}
onClose={() => setIsShowingSwitchAccount(false)}
/>
)}

<InAppMessages />
<ContainerLayout
Expand Down Expand Up @@ -171,7 +173,7 @@ export function Container() {
) : null
}
>
<Outlet />
<Outlet context={{ isShowingSwitchAccount, setIsShowingSwitchAccount }} />
</ContainerLayout>
</>
);
Expand Down
50 changes: 26 additions & 24 deletions 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 @@ -8,22 +8,24 @@ import { useWalletType } from '@app/common/use-wallet-type';
import { useCurrentAccountIndex } from '@app/store/accounts/account';
import { useFilteredBitcoinAccounts } from '@app/store/accounts/blockchain/bitcoin/bitcoin.ledger';
import { useStacksAccounts } from '@app/store/accounts/blockchain/stacks/stacks-account.hooks';
import { useHasLedgerKeys } from '@app/store/ledger/ledger.selectors';
import { Button } from '@app/ui/components/button/button';
import { Dialog, DialogProps, getHeightOffset } from '@app/ui/components/containers/dialog/dialog';
import { Dialog, DialogProps } from '@app/ui/components/containers/dialog/dialog';
import { Footer } from '@app/ui/components/containers/footers/footer';
import { Header } from '@app/ui/components/containers/headers/header';
import { virtuosoHeight, virtuosoStyles } from '@app/ui/shared/virtuoso';
import { VirtuosoWrapper } from '@app/ui/components/virtuoso';

import { AccountListUnavailable } from './components/account-list-unavailable';
import { SwitchAccountListItem } from './components/switch-account-list-item';

export interface SwitchAccountOutletContext {
isShowingSwitchAccount: boolean;
setIsShowingSwitchAccount(isShowing: boolean): void;
}

export const SwitchAccountDialog = memo(({ isShowing, onClose }: DialogProps) => {
const currentAccountIndex = useCurrentAccountIndex();
const createAccount = useCreateAccount();
const { whenWallet } = useWalletType();
const isLedger = useHasLedgerKeys();

const stacksAccounts = useStacksAccounts();
const bitcoinAccounts = useFilteredBitcoinAccounts();
const btcAddressesNum = bitcoinAccounts.length / 2;
Expand All @@ -43,13 +45,13 @@ export const SwitchAccountDialog = memo(({ isShowing, onClose }: DialogProps) =>
if (!isShowing) return null;

const accountNum = stacksAddressesNum || btcAddressesNum;
const maxAccountsShown = accountNum > 10 ? 10 : accountNum;

return (
<Dialog
header={<Header variant="dialog" title="Select account" />}
isShowing={isShowing}
onClose={onClose}
wrapChildren={false}
footer={whenWallet({
software: (
<Footer>
Expand All @@ -61,24 +63,24 @@ export const SwitchAccountDialog = memo(({ isShowing, onClose }: DialogProps) =>
ledger: null,
})}
>
<Virtuoso
height={virtuosoHeight}
style={{
...virtuosoStyles,
height: `calc(${virtuosoHeight * maxAccountsShown}px + ${getHeightOffset(true, !isLedger)}px)`,
}}
initialTopMostItemIndex={whenWallet({ ledger: 0, software: currentAccountIndex })}
totalCount={stacksAddressesNum || btcAddressesNum}
itemContent={index => (
<Box key={index} my="space.05" px="space.05">
<SwitchAccountListItem
handleClose={onClose}
currentAccountIndex={currentAccountIndex}
index={index}
/>
</Box>
)}
/>
<VirtuosoWrapper hasFooter={whenWallet({ ledger: false, software: true })}>
<Virtuoso
style={{
height: '100%',
}}
initialTopMostItemIndex={whenWallet({ ledger: 0, software: currentAccountIndex })}
totalCount={accountNum}
itemContent={index => (
<Box key={index} my="space.05" px="space.05">
<SwitchAccountListItem
handleClose={onClose}
currentAccountIndex={currentAccountIndex}
index={index}
/>
</Box>
)}
/>
</VirtuosoWrapper>
</Dialog>
);
});
4 changes: 2 additions & 2 deletions src/app/features/message-signer/message-preview-box.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ export function MessagePreviewBox({ message, hash }: MessageBoxProps) {
py="space.05"
overflowX="auto"
>
{message.split(/\r?\n/).map(line => (
<styled.span key={line} textStyle="label.01">
{message.split(/\r?\n/).map((line, index) => (
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was throwing a duplicate keys error

<styled.span key={`${line}_${index}`} textStyle="label.01">
{line}
</styled.span>
))}
Expand Down
17 changes: 12 additions & 5 deletions src/app/pages/choose-account/choose-account.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { memo, useEffect } from 'react';
import { useEffect } from 'react';
import { Outlet } from 'react-router-dom';

import { Flex, Stack, styled } from 'leather-styles/jsx';
Expand All @@ -10,10 +10,13 @@ import { useAppDetails } from '@app/common/hooks/auth/use-app-details';
import { RequesterFlag } from '@app/components/requester-flag';
import { ChooseAccountsList } from '@app/pages/choose-account/components/accounts';
import { useOnOriginTabClose } from '@app/routes/hooks/use-on-tab-closed';
import { useStacksAccounts } from '@app/store/accounts/blockchain/stacks/stacks-account.hooks';
import { LogomarkIcon } from '@app/ui/icons/logomark-icon';

export const ChooseAccount = memo(() => {
export function ChooseAccount() {
const { url } = useAppDetails();
const accounts = useStacksAccounts();
const hasConnectedStacksAccounts = accounts.length > 0;

const cancelAuthentication = useCancelAuthRequest();

Expand All @@ -34,12 +37,16 @@ export const ChooseAccount = memo(() => {
{url && <RequesterFlag requester={url.toString()} />}
<LogomarkIcon width="248px" height="58px" />
<Stack gap="space.04">
<styled.h1 textStyle="heading.05">Choose an account to connect</styled.h1>
<styled.h1 textStyle="heading.05">
{hasConnectedStacksAccounts
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 added this code as if you only have BTC connected on Ledger and then try to sign in to something it was just showing the logo and an empty screen

Now it shows:
Screenshot 2024-04-23 at 11 14 46

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe we prob need some more explanation of what's going on for users?
something like 'you are connected with ledger in bitcoin mode only, please connect stacks account as well to use this feature' or smth like that

Copy link
Contributor Author

@pete-watters pete-watters Apr 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@mica000 @markmhendrickson @fabric-8 :

Do you have any input on what copy to use on this screen?

Right now :

  • AS A USER signed in with ledger
  • AND only have BTC connected
  • WHEN you try and sign in to a site e.g. Gamma
  • THEN you see an empty screen below the leather logo like this

Screenshot 2024-04-24 at 11 23 15

I've just made a small change so that in these cases we say No connected accounts found instead of Choose an account to connect

Copy link
Collaborator

Choose a reason for hiding this comment

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

Following up here a bit late:

Ideally the user should be able to connect their wallet to any site with a Ledger account, regardless of whether they've connected with Ledger for BTC or STX.

It seems that our legacy endpoint for connecting (used by Gamma and others) is not behaving this way. However, because it's legacy (and deprecated), we don't need to update it per se.

I also assume that our newer endpoint (getAddresses) does behave this way, though I haven't tested per se. And as we implement this endpoint for mobile, we should retain this agnostic behavior, allowing users to connect regardless of Ledger connectivity by token.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for following up anyway 👍

? 'Choose an account to connect'
: 'No connected accounts found'}
</styled.h1>
</Stack>
</Stack>
<ChooseAccountsList />
{hasConnectedStacksAccounts && <ChooseAccountsList />}
</Flex>
<Outlet />
</>
);
});
}
49 changes: 22 additions & 27 deletions 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 @@ -20,8 +20,8 @@ import { useNativeSegwitAccountIndexAddressIndexZero } from '@app/store/accounts
import { useStacksAccounts } from '@app/store/accounts/blockchain/stacks/stacks-account.hooks';
import { StacksAccount } from '@app/store/accounts/blockchain/stacks/stacks-account.models';
import { AccountAvatar } from '@app/ui/components/account/account-avatar/account-avatar';
import { VirtuosoWrapper } from '@app/ui/components/virtuoso';
import { PlusIcon } from '@app/ui/icons/plus-icon';
import { virtuosoHeight, virtuosoStyles } from '@app/ui/shared/virtuoso';

interface AccountTitlePlaceholderProps {
account: StacksAccount;
Expand All @@ -43,7 +43,6 @@ const ChooseAccountItem = memo(
const btcAddress = useNativeSegwitAccountIndexAddressIndexZero(account.index);

const accountSlug = useMemo(() => slugify(`Account ${account?.index + 1}`), [account?.index]);

return (
<AccountListItemLayout
accountAddresses={<AcccountAddresses index={account.index} />}
Expand Down Expand Up @@ -71,7 +70,7 @@ const ChooseAccountItem = memo(
}
);

const AddAccountAction = memo(() => {
function AddAccountAction() {
const [component, bind] = usePressable(true);
const createAccount = useCreateAccount();

Expand All @@ -88,9 +87,9 @@ const AddAccountAction = memo(() => {
{component}
</Box>
);
});
}

export const ChooseAccountsList = memo(() => {
export function ChooseAccountsList() {
const finishSignIn = useFinishAuthRequest();
const { whenWallet } = useWalletType();
const accounts = useStacksAccounts();
Expand All @@ -110,31 +109,27 @@ 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 * maxAccountsShown}px + 50px)`,
background: token('colors.ink.background-primary'),
}}
data={accounts}
itemContent={(index, account) => (
<Box key={index} my="space.05" px="space.05">
<ChooseAccountItem
account={account}
isLoading={whenWallet({ software: selectedAccount === index, ledger: false })}
onSelectAccount={signIntoAccount}
/>
</Box>
)}
/>
<VirtuosoWrapper isPopup>
<Virtuoso
style={{
background: token('colors.ink.background-primary'),
}}
data={accounts}
itemContent={(index, account) => (
<Box key={index} my="space.05" px="space.05">
<ChooseAccountItem
account={account}
isLoading={whenWallet({ software: selectedAccount === index, ledger: false })}
onSelectAccount={signIntoAccount}
/>
</Box>
)}
/>
</VirtuosoWrapper>
</Box>
);
});
}
15 changes: 4 additions & 11 deletions src/app/pages/home/home.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { useState } from 'react';
import { Route, useNavigate } from 'react-router-dom';
import { Route, useNavigate, useOutletContext } from 'react-router-dom';

import { RouteUrls } from '@shared/route-urls';

Expand All @@ -9,7 +8,7 @@ import { useTotalBalance } from '@app/common/hooks/balance/use-total-balance';
import { useOnMount } from '@app/common/hooks/use-on-mount';
import { ActivityList } from '@app/features/activity-list/activity-list';
import { AssetsList } from '@app/features/asset-list/asset-list';
import { SwitchAccountDialog } from '@app/features/dialogs/switch-account-dialog/switch-account-dialog';
import { SwitchAccountOutletContext } from '@app/features/dialogs/switch-account-dialog/switch-account-dialog';
import { FeedbackButton } from '@app/features/feedback-button/feedback-button';
import { homePageModalRoutes } from '@app/routes/app-routes';
import { ModalBackgroundWrapper } from '@app/routes/components/modal-background-wrapper';
Expand All @@ -22,9 +21,9 @@ import { AccountActions } from './components/account-actions';
import { HomeTabs } from './components/home-tabs';

export function Home() {
const [isShowingSwitchAccount, setIsShowingSwitchAccount] = useState(false);
const { decodedAuthRequest } = useOnboardingState();

const { isShowingSwitchAccount, setIsShowingSwitchAccount } =
useOutletContext<SwitchAccountOutletContext>();
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 was investigating why we were re-rendering SwitchAccount and I found this mistake.

We were rendering it both inside of the main <Container and also inside of AccountCard.

Now it stays at Container level only and OutletContext is used to show / hide it.

It needs to stay at that level so the settings menu can also trigger it

const navigate = useNavigate();
const account = useCurrentStacksAccount();

Expand All @@ -49,12 +48,6 @@ export function Home() {
<AccountCard
name={name}
balance={totalUsdBalance}
switchAccount={
<SwitchAccountDialog
isShowing={isShowingSwitchAccount}
onClose={() => setIsShowingSwitchAccount(false)}
/>
}
toggleSwitchAccount={() => setIsShowingSwitchAccount(!isShowingSwitchAccount)}
isLoadingBnsName={isLoadingBnsName}
isLoadingBalance={isInitialLoading}
Expand Down
46 changes: 25 additions & 21 deletions ...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 @@ -6,9 +6,9 @@ import { Box } from 'leather-styles/jsx';

import { useFilteredBitcoinAccounts } from '@app/store/accounts/blockchain/bitcoin/bitcoin.ledger';
import { useStacksAccounts } from '@app/store/accounts/blockchain/stacks/stacks-account.hooks';
import { Dialog, getHeightOffset } from '@app/ui/components/containers/dialog/dialog';
import { Dialog } from '@app/ui/components/containers/dialog/dialog';
import { Header } from '@app/ui/components/containers/headers/header';
import { virtuosoHeight, virtuosoStyles } from '@app/ui/shared/virtuoso';
import { VirtuosoWrapper } from '@app/ui/components/virtuoso';

import { AccountListItem } from './account-list-item';

Expand All @@ -23,27 +23,31 @@ 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 * maxAccountsShown}px + ${getHeightOffset(true, true)}px)`,
}}
itemContent={index => (
<Box key={index} my="space.05" px="space.05">
<AccountListItem
stacksAccount={stacksAccounts[index]}
onClose={onGoBack}
index={index}
/>
</Box>
)}
totalCount={stacksAddressesNum || btcAddressesNum}
/>
<Dialog
header={<Header variant="dialog" title="My accounts" />}
isShowing
onClose={onGoBack}
wrapChildren={false}
>
<VirtuosoWrapper>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Virtuoso height is now set to 100% and the actual height is controlled by a wrapping container

<Virtuoso
style={{
height: '100%',
}}
itemContent={index => (
<Box key={index} my="space.05" px="space.05">
<AccountListItem
stacksAccount={stacksAccounts[index]}
onClose={onGoBack}
index={index}
/>
</Box>
)}
totalCount={accountNum}
/>
</VirtuosoWrapper>
</Dialog>
);
}
Loading
Loading