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

fix: blinking wallet banner, double scroll #9653

Merged
merged 4 commits into from
Aug 11, 2023
Merged

fix: blinking wallet banner, double scroll #9653

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d850aa0
fix: blinking wallet banner, double scroll
heorhi-deriv Aug 10, 2023
6771328
fix: blinking wallet banner with no wallets account
heorhi-deriv Aug 10, 2023
65f5033
refactor: remove has_wallet, add is_migrated check
heorhi-deriv Aug 10, 2023
e7fb02c
refactor: improve conditions
heorhi-deriv Aug 11, 2023
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
12 changes: 1 addition & 11 deletions packages/appstore/src/components/main-title-bar/__tests__/index.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,17 +77,7 @@ describe('MainTitleBar', () => {
expect(screen.queryByText('WalletsBanner')).not.toBeInTheDocument();
});

it('should not render WalletsBanner component if wallet feature flag is enabled and client has at least one wallet', () => {
const mock_store = mockStore({
client: { accounts: { CRW909900: { token: '12345' } }, loginid: 'CRW909900' },
feature_flags: { data: { wallet: true } },
});

render_container(mock_store);
expect(screen.queryByText('WalletsBanner')).not.toBeInTheDocument();
});

it("should render WalletsBanner component if wallet feature flag is enabled and client doesn't have any wallet", () => {
it('should render WalletsBanner component if wallet feature flag is enabled', () => {
const mock_store = mockStore({
client: { accounts: { CR123456: { token: '12345' } }, loginid: 'CR123456' },
feature_flags: { data: { wallet: true } },
Expand Down
9 changes: 3 additions & 6 deletions packages/appstore/src/components/main-title-bar/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React from 'react';
import { Text, DesktopWrapper, MobileWrapper, Tabs, Icon } from '@deriv/components';
import { useWalletsList, useFeatureFlags } from '@deriv/hooks';
import { useFeatureFlags } from '@deriv/hooks';
import { ContentFlag } from '@deriv/shared';
import { observer, useStore } from '@deriv/stores';
import AccountTypeDropdown from './account-type-dropdown';
Expand All @@ -25,9 +25,6 @@ const MainTitleBar = () => {
}, [selected_region]);

const { is_wallet_enabled } = useFeatureFlags();
const { has_wallet } = useWalletsList();

const should_show_banner = is_wallet_enabled && !has_wallet;

// TODO: Remove this when we have BE API ready
removeAllNotificationMessages(true);
Expand All @@ -40,7 +37,7 @@ const MainTitleBar = () => {
return (
<React.Fragment>
<DesktopWrapper>
{should_show_banner && <WalletsBanner />}
{is_wallet_enabled && <WalletsBanner />}
<div className='main-title-bar'>
<div className='main-title-bar__right'>
<Text size='m' weight='bold' color='prominent'>
Expand All @@ -53,7 +50,7 @@ const MainTitleBar = () => {
</div>
</DesktopWrapper>
<MobileWrapper>
{should_show_banner && <WalletsBanner />}
{is_wallet_enabled && <WalletsBanner />}
<Text weight='bold' className='main-title-bar__text' color='prominent'>
{localize("Trader's Hub")}
</Text>
Expand Down
5 changes: 4 additions & 1 deletion packages/appstore/src/components/routes/routes.tsx
View file
Open in desktop
Copy link
Contributor Author

Choose a reason for hiding this comment

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

in Routes component i decided to leave useWalletsList hook, because useWalletMigration will be removed after some time.
Here we basically check:

  1. If the FF is enabled and we have no wallets - we show legacy TH with banner.
  2. If the FF is enabled and we have wallets - we show WalletsModule.
  3. If the FF is disabled we always show legacy TH without banner.

Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import * as React from 'react';
import { Loading } from '@deriv/components';
import { useFeatureFlags, useWalletsList } from '@deriv/hooks';
import { observer } from '@deriv/stores';
import { Localize, localize } from '@deriv/translations';
Expand All @@ -10,10 +11,12 @@ import RouteWithSubroutes from './route-with-sub-routes.jsx';

const Routes: React.FC = observer(() => {
const { is_wallet_enabled } = useFeatureFlags();
const { has_wallet } = useWalletsList();
const { has_wallet, isLoading } = useWalletsList();
heorhi-deriv marked this conversation as resolved.
Show resolved Hide resolved

const should_show_wallets = is_wallet_enabled && has_wallet;

if (isLoading) return <Loading />;

return (
<React.Suspense
fallback={
Expand Down
6 changes: 3 additions & 3 deletions packages/appstore/src/components/wallets-banner/wallets-banner.tsx
View file
Open in desktop
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here i slightly refactored if conditions to avoid blinking of wallet banner in some scenarios.
because earlier we could show WalletsBannerReady when state is undefined during the first loading of the App.
Now we return null if the state is undefined or the user is_ineligible.

Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,17 @@ import { useWalletMigration } from '@deriv/hooks';
import { observer, useStore } from '@deriv/stores';

const WalletsBanner = observer(() => {
const { is_eligible, is_failed, is_in_progress, is_ineligible } = useWalletMigration();
const { is_eligible, is_failed, is_in_progress, is_migrated } = useWalletMigration();
const { traders_hub } = useStore();
const { is_eu_user } = traders_hub;

if (is_ineligible) return null;
if (is_migrated) return <WalletsBannerReady is_eu={is_eu_user} />;

if (is_eligible || is_failed) return <WalletsBannerUpgrade />;

if (is_in_progress) return <WalletsBannerUpgrading is_eu={is_eu_user} />;

return <WalletsBannerReady is_eu={is_eu_user} />;
return null;
});

export default WalletsBanner;
2 changes: 1 addition & 1 deletion packages/appstore/src/modules/wallets/wallets.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
.wallets-module {
display: flex;
min-height: 92.8vh;
min-height: calc(100vh - 84px);
heorhi-deriv marked this conversation as resolved.
Show resolved Hide resolved
background-color: var(--general-section-1);

@include mobile {
Expand Down