feat: Table loading skeleton #663
Conversation
📝 WalkthroughWalkthroughThis pull request introduces a new Changes
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
warning @pluralsh/eslint-config-typescript > [email protected]: This version is no longer supported. Please see https://eslint.org/version-support for other options. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Visit the preview URL for this PR (updated for commit 4920fee): https://pluralsh-design--pr663-marcin-prod-2905-add-cl6pxhlu.web.app (expires Thu, 28 Nov 2024 07:59:25 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 784914c934330f8d0a9fd65c68898b3988262b7d |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (4)
src/components/table/Skeleton.tsx (1)
11-29: Improve animation consistency and component flexibilityThe styled component implementation has a few areas for improvement:
- Make the animation distance match the background size:
- '0%': { backgroundPosition: '-250px 0' }, - '100%': { backgroundPosition: '250px 0' }, + '0%': { backgroundPosition: '-500px 0' }, + '100%': { backgroundPosition: '500px 0' },
- Consider making dimensions configurable:
- maxWidth: '400px', + maxWidth: ({ maxWidth = '400px' }) => maxWidth, width: '100%', span: { borderRadius: theme.borderRadiuses.medium, - maxWidth: '400px', + maxWidth: 'inherit', - width: 'unset', - minWidth: '150px', + width: ({ width = '150px' }) => width,
- Consider extracting animation duration to a prop:
- animation: 'moving-gradient 2s infinite linear forwards', + animation: ({ duration = '2s' }) => + `moving-gradient ${duration} infinite linear forwards`,src/stories/Table.stories.tsx (1)
378-386: Consider enhancing the Loading story demonstrationWhile the basic loading state is implemented correctly, consider these improvements to better showcase the feature:
- Add the
loadingSkeletonRowsprop to demonstrate customizable number of skeleton rows- Add a knob/control for the loading state to allow toggling between loading and loaded states
- Document the loading-related props in the story
Example enhancement:
export const Loading = Template.bind({}) Loading.args = { fillLevel: 0, width: '900px', height: '400px', data: [], columns, loading: true, + loadingSkeletonRows: 5, // Show 5 skeleton rows while loading } +Loading.parameters = { + docs: { + description: { + story: ` + The Table component supports a loading state that displays skeleton rows. + - Use \`loading\` prop to toggle the loading state + - Use \`loadingSkeletonRows\` prop to customize the number of skeleton rows + `, + }, + }, +}src/components/table/Table.tsx (2)
381-404: Consider memoizing the columns iterationWhile the implementation is correct, we can optimize the columns iteration to prevent unnecessary re-renders.
-{columns.map((_, j) => ( +{useMemo(() => columns.map((_, j) => ( <Td key={j} $fillLevel={fillLevel} $firstRow={i === 0} $padCells={padCells} $loose={loose} $stickyColumn={stickyColumn} $truncateColumn={false} > <Skeleton /> </Td> -))} +)), [columns, fillLevel, padCells, loose, stickyColumn])}
418-498: Consider adding error boundaries for row renderingThe row rendering logic handles various states well, but could benefit from error boundaries to prevent the entire table from crashing if individual row rendering fails.
Consider wrapping the row rendering logic in an error boundary component to gracefully handle rendering failures:
class RowErrorBoundary extends React.Component { state = { hasError: false }; static getDerivedStateFromError(error) { return { hasError: true }; } render() { if (this.state.hasError) { return <Tr><Td colSpan={100}>Error rendering row</Td></Tr>; } return this.props.children; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
src/components/table/Skeleton.tsx(1 hunks)src/components/table/Table.tsx(5 hunks)src/stories/Table.stories.tsx(1 hunks)
🔇 Additional comments (6)
src/components/table/Skeleton.tsx (1)
1-29: Verify Table component integration
Let's verify the integration of this Skeleton component with the Table component.
✅ Verification successful
Skeleton component is properly integrated with Table
The verification shows that the Skeleton component is correctly integrated with the Table component:
- It's properly imported in
Table.tsx - The Table component has the necessary loading-related props:
loadingandloadingSkeletonRows - The Skeleton is used within the Table's loading state, rendering multiple skeleton rows based on
loadingSkeletonRowsprop (default: 10)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check how the Skeleton component is imported and used in the Table component
rg -A 5 "import.*Skeleton.*from.*['\"]\.\/Skeleton" src/components/table/
# Check for the usage of loading props in the Table component
ast-grep --pattern 'interface TableProps {
$$$
loading?: $_
$$$
}'
Length of output: 289
Script:
#!/bin/bash
# Check how the Skeleton component is imported and used in the Table component
rg -A 5 "import.*Skeleton" src/components/table/
# Check for any usage of Skeleton component in Table-related files
rg -A 5 "Skeleton" src/components/table/
# Check for loading prop in Table component
rg -A 5 "loading.*?:" src/components/table/
# Find Table component interface or type definitions
ast-grep --pattern 'interface Table {
$$$
}'
# Also check for TableProps
ast-grep --pattern 'interface TableProps {
$$$
}'
Length of output: 3659
src/stories/Table.stories.tsx (2)
378-386: LGTM - Story implementation follows established patterns
The Loading story maintains consistency with other stories in terms of:
- Story structure and binding
- Component dimensions
- Basic table properties
378-386: Verify the loading feature implementation
Let's verify the complete implementation of the loading feature:
✅ Verification successful
Loading feature implementation is complete and well-structured
The loading state implementation is properly handled with:
- A dedicated Skeleton component with styled animation effects
- Table component properly implements loading state with configurable skeleton rows
- Loading state controls visibility of content and empty state
- Loading story demonstrates the feature
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for the Skeleton component implementation and related changes
# Check for Skeleton component
echo "Checking for Skeleton component implementation..."
fd -e tsx -e ts "Skeleton" src/components/table/
# Check for Table component changes
echo "Checking for Table component changes..."
rg "loading.*?:|loadingSkeletonRows.*?:" src/components/table/Table.tsx
# Check for test coverage
echo "Checking for test coverage..."
fd -e test.tsx -e test.ts "Table|Skeleton" src/components/table/
Length of output: 541
Script:
#!/bin/bash
# Let's examine the implementation details
# Check Skeleton component implementation
echo "=== Skeleton Component Implementation ==="
cat src/components/table/Skeleton.tsx
# Check Table component's loading handling
echo -e "\n=== Table Component Loading Logic ==="
rg -B 2 -A 4 "loading[?]:" src/components/table/Table.tsx
# Check if there are any tests specifically for loading state
echo -e "\n=== Loading-related Test Cases ==="
rg "loading.*true" src/components/table/ -A 3
Length of output: 1367
Script:
#!/bin/bash
# Let's check how the loading state is actually used in the Table component
# Check Table component's usage of loading props
echo "=== Table Component Loading Implementation ==="
rg -B 5 -A 10 "loading[^:]" src/components/table/Table.tsx
# Check for Skeleton component usage
echo -e "\n=== Skeleton Usage in Table ==="
rg "Skeleton" src/components/table/Table.tsx -A 3
# Check stories for loading state examples
echo -e "\n=== Loading Stories Implementation ==="
rg "loading.*true" src/stories/Table.stories.tsx -A 3
Length of output: 3153
src/components/table/Table.tsx (3)
269-272: LGTM! Efficient memoization of skeleton rows
The skeleton rows array is properly memoized to prevent unnecessary recreations.
Line range hint 513-517: LGTM! Proper empty state handling
The empty state logic correctly handles the interaction between loading and empty states.
42-42: Verify the Skeleton component implementation
The new Skeleton component is imported but its implementation is not included in this PR. Please ensure that:
- The Skeleton component is properly implemented and tested
- It follows the design system's guidelines for loading states
Also applies to: 54-55
✅ Verification successful
Skeleton component is properly implemented
The Skeleton component exists and is correctly implemented with:
- Styled-components integration with theme support
- Proper animation using CSS keyframes for loading effect
- Appropriate styling for a loading state placeholder
- Design system integration through theme variables (colors, border-radius)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if Skeleton component exists and verify its implementation
fd -e tsx -e ts "Skeleton" src/components/table/
Length of output: 82
Script:
#!/bin/bash
# Check the implementation of the Skeleton component
cat src/components/table/Skeleton.tsx
Length of output: 835
maciaszczykm
deleted the
marcin/prod-2905-add-loadingskeleton-state-for-table-component
branch
|
🎉 This PR is included in version 3.80.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Summary by CodeRabbit
New Features
Skeletoncomponent for visually appealing loading placeholders.Tablecomponent, allowing it to display skeleton rows while data is being fetched.Tablecomponent in Storybook to demonstrate the loading state.Bug Fixes