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

fix(hub-react): adapted skeletons display to reduce cls kpi #14275

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,27 @@ import {
ShellContext,
useOvhTracking,
} from '@ovh-ux/manager-react-shell-client';
import { OsdsLink, OsdsSkeleton } from '@ovhcloud/ods-components/react';
import { OsdsLink } from '@ovhcloud/ods-components/react';
import {
OdsHTMLAnchorElementTarget,
OdsHTMLAnchorElementRel,
} from '@ovhcloud/ods-common-core';
import { useFetchHubBanner } from '@/data/hooks/banner/useBanner';
import { useHubContext } from '@/pages/layout/context';

const DEFAULT_TRACKING = ['hub', 'dashboard', 'event-banner'];

export default function Banner() {
const { environment } = useContext(ShellContext);
const locale = environment.getUserLocale();
const { trackClick } = useOvhTracking();
const { isLoading, isFreshCustomer } = useHubContext();

const { data: banner, isPending: isLoading } = useFetchHubBanner(locale);
const { data: banner, isPending } = useFetchHubBanner(locale);

return (
<>
{isLoading && <OsdsSkeleton data-testid="banner_skeleton" inline />}
{!isLoading && banner && (
{!isLoading && !isPending && !isFreshCustomer && banner && (
<OsdsLink
className="mb-4"
onClick={() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,6 @@ vi.mock('@ovh-ux/manager-react-shell-client', async (importOriginal) => {
});

describe('Banner.component', () => {
it('should display a skeleton while loading', async () => {
const { getByTestId } = renderComponent();

expect(getByTestId('banner_skeleton')).not.toBeNull();
});

it('should not display the banner if none is returned by the api', async () => {
loading = false;
const { queryByTestId } = renderComponent();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Suspense, useContext, useMemo } from 'react';
import { lazy, Suspense, useContext, useMemo } from 'react';
import {
OsdsChip,
OsdsIcon,
Expand All @@ -14,6 +14,7 @@ import {
ODS_TEXT_SIZE,
ODS_ICON_SIZE,
ODS_TEXT_COLOR_HUE,
ODS_SKELETON_SIZE,
} from '@ovhcloud/ods-components';
import {
ODS_THEME_COLOR_INTENT,
Expand All @@ -37,15 +38,18 @@ import {
} from '@/data/api/apiOrder/apiOrder.constants';
import useDateFormat from '@/hooks/dateFormat/useDateFormat';
import { LastOrderTrackingResponse, OrderHistory } from '@/types/order.type';
import { Skeletons } from '@/components/skeletons/Skeletons.component';
// FIXME: lazy load these comoponents
import TileError from '@/components/tile-error/TileError.component';
import { useHubContext } from '@/pages/layout/context';

const TileError = lazy(() =>
import('@/components/tile-error/TileError.component'),
);

export default function HubOrderTracking() {
const { t } = useTranslation('hub/order');
const { isLoading, isFreshCustomer } = useHubContext();
const {
data: orderDataResponse,
isLoading,
isLoading: isLastOrderLoading,
error,
refetch,
} = useFetchLastOrder();
Expand Down Expand Up @@ -118,25 +122,30 @@ export default function HubOrderTracking() {
);

return (
<OsdsTile
className="block p-1 bg-[var(--ods-color-primary-200)] p-6"
variant={ODS_TILE_VARIANT.ghost}
inline
>
<div className="bg-500 !flex flex-col gap-1 items-center justifier-center">
<OsdsText
color={ODS_THEME_COLOR_INTENT.primary}
level={ODS_THEME_TYPOGRAPHY_LEVEL.heading}
className="block"
size={ODS_TEXT_SIZE._300}
>
{t('hub_order_tracking_title')}
</OsdsText>
(isLoading || !isFreshCustomer) && (
<OsdsTile
className="block p-1 bg-[var(--ods-color-primary-200)] p-6"
variant={ODS_TILE_VARIANT.ghost}
inline
>
<div className="bg-500 !flex flex-col gap-1 items-center justifier-center">
<OsdsText
color={ODS_THEME_COLOR_INTENT.primary}
level={ODS_THEME_TYPOGRAPHY_LEVEL.heading}
className="block"
size={ODS_TEXT_SIZE._300}
>
{t('hub_order_tracking_title')}
</OsdsText>

{isLoading ? (
<Skeletons />
) : (
<>
{isLoading || isLastOrderLoading ? (
<OsdsSkeleton
data-testid="order_link_skeleton"
className="font-bold text-right flex flex-col items-center justifier-center mb-6"
inline
size={ODS_SKELETON_SIZE.sm}
/>
) : (
<Suspense fallback={<OsdsSkeleton inline randomized />}>
<Await
resolve={orderTrackingLinkAsync}
Expand Down Expand Up @@ -165,37 +174,59 @@ export default function HubOrderTracking() {
)}
/>
</Suspense>

<div className="mb-6 flex justify-center gap-3 items-center flex-wrap">
<OsdsText
level={ODS_THEME_TYPOGRAPHY_LEVEL.body}
hue={ODS_TEXT_COLOR_HUE._800}
color={ODS_THEME_COLOR_INTENT.primary}
className="block font-bold mr-1"
>
{format(new Date(currentStatus.date))}
</OsdsText>
<OsdsText
level={ODS_THEME_TYPOGRAPHY_LEVEL.body}
hue={ODS_TEXT_COLOR_HUE._800}
color={ODS_THEME_COLOR_INTENT.primary}
className="block mr-1"
)}
<div className="mb-6 flex justify-center gap-3 items-center flex-wrap">
{isLoading || isLastOrderLoading ? (
<OsdsSkeleton
data-testid="order_info_skeleton"
className="font-bold text-right flex flex-col items-center justifier-center"
inline
size={ODS_SKELETON_SIZE.sm}
/>
) : (
<>
<OsdsText
level={ODS_THEME_TYPOGRAPHY_LEVEL.body}
hue={ODS_TEXT_COLOR_HUE._800}
color={ODS_THEME_COLOR_INTENT.primary}
className="block font-bold mr-1"
>
{format(new Date(currentStatus.date))}
</OsdsText>
<OsdsText
level={ODS_THEME_TYPOGRAPHY_LEVEL.body}
hue={ODS_TEXT_COLOR_HUE._800}
color={ODS_THEME_COLOR_INTENT.primary}
className="block mr-1"
>
{t(`order_tracking_history_${currentStatus.label}`)}
</OsdsText>
<OsdsIcon
size={ODS_ICON_SIZE.xxs}
color={ODS_THEME_COLOR_INTENT.text}
name={
!ERROR_STATUS.includes(currentStatus.label) &&
!isWaitingPayment
? ODS_ICON_NAME.OK
: ODS_ICON_NAME.CLOSE
}
></OsdsIcon>
</>
anooparveti marked this conversation as resolved.
Show resolved Hide resolved
)}
</div>
<div>
{isLoading || isLastOrderLoading ? (
<div className="mb-6 flex justify-center gap-3 items-center flex-wrap">
<OsdsSkeleton
data-testid="orders_link_skeleton"
inline
size={ODS_SKELETON_SIZE.sm}
/>
</div>
) : (
<Suspense
fallback={<OsdsSkeleton inline size={ODS_SKELETON_SIZE.sm} />}
>
{t(`order_tracking_history_${currentStatus.label}`)}
</OsdsText>
<OsdsIcon
size={ODS_ICON_SIZE.xxs}
color={ODS_THEME_COLOR_INTENT.text}
name={
!ERROR_STATUS.includes(currentStatus.label) &&
!isWaitingPayment
? ODS_ICON_NAME.OK
: ODS_ICON_NAME.CLOSE
}
></OsdsIcon>
</div>
<div>
<Suspense fallback={<OsdsSkeleton inline randomized />}>
<Await
resolve={ordersTrackingLinkAsync}
children={(ordersTrackingLink: string) => (
Expand All @@ -218,10 +249,10 @@ export default function HubOrderTracking() {
)}
/>
</Suspense>
</div>
</>
)}
</div>
</OsdsTile>
)}
</div>
</div>
</OsdsTile>
)
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,7 @@ const { refetch } = vi.hoisted(() => ({

const trackClickMock = vi.fn();

vi.mock('../skeletons/Skeletons.component', () => ({
Skeletons: () => <div data-testid="tile-skeleton"></div>,
}));

vi.mock('../tile-error/TileError.component', () => ({
vi.mock('@/components/tile-error/TileError.component', () => ({
default: () => <div data-testid="tile-error"></div>,
}));

Expand Down Expand Up @@ -85,22 +81,26 @@ describe('HubOrderTracking Component', async () => {
});
});

it('displays loading skeleton when isLoading is true', () => {
it('displays loading skeletons when isLoading is true', () => {
useFetchLastOrderMockValue.isLoading = true;

render(<HubOrderTracking />);

const skeleton = screen.getByTestId('tile-skeleton');
const orderLinkSkeleton = screen.getByTestId('order_link_skeleton');
const orderInfoSkeleton = screen.getByTestId('order_info_skeleton');
const ordersLinkSkeleton = screen.getByTestId('orders_link_skeleton');

expect(skeleton).toBeInTheDocument();
expect(orderLinkSkeleton).toBeInTheDocument();
expect(orderInfoSkeleton).toBeInTheDocument();
expect(ordersLinkSkeleton).toBeInTheDocument();
});

it('displays TileError when there is an error', () => {
it('displays TileError when there is an error', async () => {
useFetchLastOrderMockValue.error = true;

render(<HubOrderTracking />);

const tileError = screen.getByTestId('tile-error');
const tileError = await screen.findByTestId('tile-error');
expect(tileError).toBeInTheDocument();
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ import { useFetchHubSupport } from '@/data/hooks/apiHubSupport/useHubSupport';
import { SUPPORT_URLS } from './HubSupport.constants';
import { HubSupportHelp } from './hub-support-help/HubSupportHelp.component';
import { HubSupportTable } from './hub-support-table/HubSupportTable.component';
import { Skeletons } from '../skeletons/Skeletons.component';
import TileError from '../tile-error/TileError.component';
import { Skeletons } from '@/components/skeletons/Skeletons.component';

export default function HubSupport() {
const { t } = useTranslation('hub/support');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export default function TileGridSkeleton() {
<OsdsSkeleton
data-testid="tile_grid_title_skeleton"
size={ODS_SKELETON_SIZE.md}
className="my-6"
/>
<div
className="grid grid-cols-1 md:grid-cols-2 lg:grid-cols-3 gap-5 pt-3 mb-4"
Expand All @@ -17,6 +18,9 @@ export default function TileGridSkeleton() {
<TileSkeleton />
<TileSkeleton />
<TileSkeleton />
<TileSkeleton />
<TileSkeleton />
<TileSkeleton />
</div>
</>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@ import { render } from '@testing-library/react';
import TileGridSkeleton from '@/components/tile-grid-skeleton/TileGridSkeleton.component';

describe('TileGridSkeleton', () => {
it('should display a skeleton for the title and 4 inner skeletons', async () => {
it('should display a skeleton for the title and 6 inner skeletons', async () => {
const { getByTestId } = render(<TileGridSkeleton />);

expect(getByTestId('tile_grid_title_skeleton')).not.toBeNull();
const contentContainer = getByTestId('tile_grid_content_skeletons');
expect(contentContainer).not.toBeNull();
expect(contentContainer.children.length).toBe(3);
expect(contentContainer.children.length).toBe(6);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,18 @@ export default function TileSkeleton() {
className="flex mb-2 gap-4 justify-between ovh-manager-hub-tile__header"
data-testid="tile_skeleton_header"
>
<OsdsSkeleton inline size={ODS_SKELETON_SIZE.sm} />
<OsdsSkeleton className="mb-8" inline size={ODS_SKELETON_SIZE.sm} />
<OsdsSkeleton
className="mb-8 float-right"
inline
size={ODS_SKELETON_SIZE.xs}
className="float-right"
/>
</div>
<div data-testid="tile_skeleton_content">
<OsdsSkeleton inline randomized className="list-none list-item" />
<OsdsSkeleton inline randomized className="list-none list-item" />
<OsdsSkeleton inline randomized className="list-none list-item" />
<OsdsSkeleton inline randomized className="list-none list-item" />
</div>
</div>
</OsdsTile>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { describe, expect, it } from 'vitest';
import { render } from '@testing-library/react';
import TileSkeleton from '@/components/tile-grid-skeleton/tile-skeleton/TileSkeleton.component';

describe('TileGridSkeleton', () => {
describe('TileSkeleton', () => {
it('should display a skeleton for the title and 4 inner skeletons', async () => {
const { getByTestId } = render(<TileSkeleton />);

Expand All @@ -12,6 +12,6 @@ describe('TileGridSkeleton', () => {
expect(headerContainer.children.length).toBe(2);
const contentContainer = getByTestId('tile_skeleton_content');
expect(contentContainer).not.toBeNull();
expect(contentContainer.children.length).toBe(2);
expect(contentContainer.children.length).toBe(4);
});
});
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import { useQuery } from '@tanstack/react-query';
import { DefinedInitialDataOptions, useQuery } from '@tanstack/react-query';
import { AxiosError } from 'axios';
import { getBillingServices } from '@/data/api/billingServices';
import { HubBillingServices } from '@/billing/types/billingServices.type';

export const useFetchHubBillingServices = () =>
export const useFetchHubBillingServices = (
options?: Partial<DefinedInitialDataOptions<HubBillingServices, AxiosError>>,
) =>
useQuery<HubBillingServices, AxiosError>({
...options,
queryKey: ['getHubBillingServices'],
queryFn: getBillingServices,
retry: 0,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
import { useQuery } from '@tanstack/react-query';
import { DefinedInitialDataOptions, useQuery } from '@tanstack/react-query';
import { AxiosError } from 'axios';
import { getBills } from '@/data/api/bills';
import { Bills } from '@/types/bills.type';

export const useFetchHubBills = (period: number) =>
export const useFetchHubBills = (
period: number,
options?: Partial<DefinedInitialDataOptions<Bills, AxiosError>>,
) =>
useQuery<Bills, AxiosError>({
...options,
queryKey: ['getHubBills', period],
queryFn: () => getBills(period),
retry: 0,
Expand Down
Loading
Loading