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

Feat: Implement loading skeleton small screen width #43652

Merged
merged 13 commits into from
Jun 24, 2024
Merged

Feat: Implement loading skeleton small screen width #43652

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
20dc2de
Feat: Implement loading skeleton small screen width
truph01 Jun 13, 2024
038fead
Fix: Lint
truph01 Jun 13, 2024
db3f19e
Fix: Lint
truph01 Jun 13, 2024
e211f53
Fix: Merge main
truph01 Jun 14, 2024
43cde3e
Fix: Update SEARCH_SKELETON_VIEW_ITEM_HEIGHT to 100
truph01 Jun 14, 2024
d3b95e3
Fix: Update row gap to 12px
truph01 Jun 14, 2024
4a7e860
Fix: Merge main branch
truph01 Jun 17, 2024
284e32a
Fix: Comment
truph01 Jun 17, 2024
fc1a1fa
Fix: Update radius
truph01 Jun 17, 2024
c33c891
Fix: Change gap to 8
truph01 Jun 20, 2024
269df4f
Fix: Update height to 108
truph01 Jun 21, 2024
1c61053
Fix: Remove test logic
truph01 Jun 21, 2024
5172df9
Fix: Add key prop to view
truph01 Jun 22, 2024
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
1 change: 1 addition & 0 deletions src/CONST.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1252,6 +1252,7 @@ const CONST = {
},
CENTRAL_PANE_ANIMATION_HEIGHT: 200,
LHN_SKELETON_VIEW_ITEM_HEIGHT: 64,
SEARCH_SKELETON_VIEW_ITEM_HEIGHT: 90,
truph01 marked this conversation as resolved.
Show resolved Hide resolved
EXPENSIFY_PARTNER_NAME: 'expensify.com',
EMAIL: {
ACCOUNTING: 'accounting@expensify.com',
Expand Down
30 changes: 17 additions & 13 deletions src/components/Skeletons/ItemListSkeletonView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React, {useMemo, useState} from 'react';
import {View} from 'react-native';
import type {StyleProp, ViewStyle} from 'react-native';
import SkeletonViewContentLoader from '@components/SkeletonViewContentLoader';
import useTheme from '@hooks/useTheme';
import useThemeStyles from '@hooks/useThemeStyles';
Expand All @@ -9,9 +10,11 @@ type ListItemSkeletonProps = {
shouldAnimate?: boolean;
renderSkeletonItem: (args: {itemIndex: number}) => React.ReactNode;
fixedNumItems?: number;
itemViewStyle?: StyleProp<ViewStyle>;
itemViewHeight?: number;
};

function ItemListSkeletonView({shouldAnimate = true, renderSkeletonItem, fixedNumItems}: ListItemSkeletonProps) {
function ItemListSkeletonView({shouldAnimate = true, renderSkeletonItem, fixedNumItems, itemViewStyle = {}, itemViewHeight = CONST.LHN_SKELETON_VIEW_ITEM_HEIGHT}: ListItemSkeletonProps) {
const theme = useTheme();
const themeStyles = useThemeStyles();

Expand All @@ -20,20 +23,21 @@ function ItemListSkeletonView({shouldAnimate = true, renderSkeletonItem, fixedNu
const items = [];
for (let i = 0; i < numItems; i++) {
items.push(
<SkeletonViewContentLoader
key={`skeletonViewItems${i}`}
animate={shouldAnimate}
height={CONST.LHN_SKELETON_VIEW_ITEM_HEIGHT}
backgroundColor={theme.skeletonLHNIn}
foregroundColor={theme.skeletonLHNOut}
style={themeStyles.mr5}
>
{renderSkeletonItem({itemIndex: i})}
</SkeletonViewContentLoader>,
<View style={[themeStyles.mr5, itemViewStyle]}>
truph01 marked this conversation as resolved.
Show resolved Hide resolved
<SkeletonViewContentLoader
key={`skeletonViewItems${i}`}
animate={shouldAnimate}
height={itemViewHeight}
backgroundColor={theme.skeletonLHNIn}
foregroundColor={theme.skeletonLHNOut}
>
{renderSkeletonItem({itemIndex: i})}
</SkeletonViewContentLoader>
</View>,
);
}
return items;
}, [numItems, shouldAnimate, theme, themeStyles, renderSkeletonItem]);
}, [numItems, shouldAnimate, theme, themeStyles, renderSkeletonItem, itemViewHeight, itemViewStyle]);

return (
<View
Expand All @@ -43,7 +47,7 @@ function ItemListSkeletonView({shouldAnimate = true, renderSkeletonItem, fixedNu
return;
}

const newNumItems = Math.ceil(event.nativeEvent.layout.height / CONST.LHN_SKELETON_VIEW_ITEM_HEIGHT);
const newNumItems = Math.ceil(event.nativeEvent.layout.height / itemViewHeight);
if (newNumItems === numItems) {
return;
}
Expand Down
88 changes: 87 additions & 1 deletion src/components/Skeletons/TableListItemSkeleton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,103 @@
import React from 'react';
import {Rect} from 'react-native-svg';
import {Circle, Rect} from 'react-native-svg';
import useThemeStyles from '@hooks/useThemeStyles';
import useWindowDimensions from '@hooks/useWindowDimensions';
import CONST from '@src/CONST';
import ItemListSkeletonView from './ItemListSkeletonView';

type TableListItemSkeletonProps = {
shouldAnimate?: boolean;
fixedNumItems?: number;
};

const circleRadius = 8;
const padding = 12;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the padding on mobile for these cards is 12 vertical, and 16 horizontal.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@shawnborton Can you share the design about the "padding" you mentioned above?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is what the padding should look like:
CleanShot 2024-06-14 at 17 28 36@2x

Copy link
Contributor

Choose a reason for hiding this comment

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

And there should be an 8px gap between the content at the top of the row and the content at the bottom:
CleanShot 2024-06-14 at 17 29 07@2x

const barHeight = '10';
const shortBarWidth = '40';
const longBarWidth = '120';

function TableListItemSkeleton({shouldAnimate = true, fixedNumItems}: TableListItemSkeletonProps) {
const styles = useThemeStyles();
const {windowWidth, isSmallScreenWidth} = useWindowDimensions();
if (isSmallScreenWidth) {
return (
<ItemListSkeletonView
itemViewHeight={CONST.SEARCH_SKELETON_VIEW_ITEM_HEIGHT}
itemViewStyle={[styles.highlightBG, styles.mv2, styles.br3, styles.mr3, styles.ml3]}
shouldAnimate={shouldAnimate}
fixedNumItems={fixedNumItems}
renderSkeletonItem={() => (
<>
<Circle
cx={padding + circleRadius}
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of doing any fancy math here, can we just say that this should be 16x16?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The fancy math will help me when debugging. I will remove these once we do not have a design change any more. Also, in this case, padding + circleRadius = 16

cy={padding + circleRadius}
r={circleRadius}
/>

<Rect
x={padding + circleRadius * 2 + 4}
y={padding + circleRadius - 2}
width={windowWidth * 0.2}
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we just give this a fixed width of 40?

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 updated. Here is result:

image

Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't match the Figma design though. Can you double check? The receipt icon is 36x40.

height={4}
/>
<Circle
cx={padding + circleRadius * 2 + 4 + windowWidth * 0.2 + circleRadius * 2 + circleRadius}
cy={padding + circleRadius}
r={circleRadius}
/>

<Rect
x={padding + circleRadius * 2 + 4 + windowWidth * 0.2 + circleRadius * 2 + circleRadius * 2 + 4}
y={padding + circleRadius - 2}
width={windowWidth * 0.2}
height={4}
/>
<Rect
x={windowWidth - padding * 3 - windowWidth * 0.2}
y={padding + circleRadius - 12}
width={windowWidth * 0.2}
height={24}
rx={12}
ry={12}
/>

<Rect
x={padding}
y={CONST.SEARCH_SKELETON_VIEW_ITEM_HEIGHT - padding - 36}
width={36}
height={36}
rx={6}
ry={6}
/>
<Rect
x={padding + 36 + circleRadius}
y={CONST.SEARCH_SKELETON_VIEW_ITEM_HEIGHT - padding - 36 + 18 - circleRadius - 2}
width={windowWidth * 0.4}
height={circleRadius}
/>
<Rect
x={padding + 36 + circleRadius}
y={CONST.SEARCH_SKELETON_VIEW_ITEM_HEIGHT - padding - 36 + 18 + 2}
width={windowWidth * 0.2}
height={circleRadius}
/>
<Rect
x={windowWidth - padding * 3 - windowWidth * 0.3}
y={CONST.SEARCH_SKELETON_VIEW_ITEM_HEIGHT - padding - 36 + 18 - circleRadius - 2}
width={windowWidth * 0.3}
height={circleRadius}
/>
<Rect
x={windowWidth - padding * 3 - windowWidth * 0.2}
y={CONST.SEARCH_SKELETON_VIEW_ITEM_HEIGHT - padding - 36 + 18 + 2}
width={windowWidth * 0.2}
height={circleRadius}
/>
</>
)}
/>
);
}
return (
<ItemListSkeletonView
shouldAnimate={shouldAnimate}
Expand Down
Loading