feat(apollo-react): support skeleton loader in BaseNode#240
feat(apollo-react): support skeleton loader in BaseNode#240rahul-sharma-uipath wants to merge 1 commit intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
There was a problem hiding this comment.
Pull request overview
Adds a skeleton/loading rendering mode to the Canvas BaseNode so nodes can display a shimmer placeholder while their content is loading.
Changes:
- Extends
BaseNodeDatawith aloading?: booleanflag to enable a skeleton state. - Implements an early-return loading render path in
BaseNodethat shows a skeleton in place of the icon area. - Introduces
BaseSkeletonIconstyling and a new Storybook story to showcase loading across shapes.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/apollo-react/src/canvas/components/BaseNode/BaseNode.types.ts | Adds loading?: boolean to the node data contract. |
| packages/apollo-react/src/canvas/components/BaseNode/BaseNode.tsx | Adds a loading/skeleton render branch for nodes. |
| packages/apollo-react/src/canvas/components/BaseNode/BaseNode.styles.ts | Adds BaseSkeletonIcon styled wrapper around ApSkeleton. |
| packages/apollo-react/src/canvas/components/BaseNode/BaseNode.stories.tsx | Adds a Loading story and manifest/node setup to demo the skeleton state. |
packages/apollo-react/src/canvas/components/BaseNode/BaseNode.styles.ts
Outdated
Show resolved
Hide resolved
| // Skeleton loading state from node data | ||
| if (data.loading) { | ||
| const skeletonShape = display.shape ?? 'square'; | ||
|
|
||
| return ( | ||
| <div | ||
| ref={containerRef} | ||
| style={{ position: 'relative' }} | ||
| onMouseEnter={handleMouseEnter} | ||
| onMouseLeave={handleMouseLeave} | ||
| onFocus={handleFocus} | ||
| onBlur={handleBlur} | ||
| > | ||
| <BaseContainer | ||
| selected={selected} | ||
| shape={skeletonShape} | ||
| className={interactionState} | ||
| interactionState={interactionState} | ||
| width={width} | ||
| height={height} | ||
| > | ||
| {/* Shimmer skeleton replacing the icon area */} | ||
| <BaseSkeletonIcon | ||
| variant="rectangle" | ||
| shape={skeletonShape} | ||
| nodeHeight={height} | ||
| nodeWidth={width} | ||
| /> | ||
| </BaseContainer> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
The data.loading early-return skips rendering handleElements. This breaks the file’s own invariant (“Always render all handles so they register…”) and can cause missing/incorrect edge anchoring or SmartHandle registration while the node is loading. Consider always rendering the handles (e.g., render handleElements with visible={false} / hidden visuals) even in loading state.
packages/apollo-react/src/canvas/components/BaseNode/BaseNode.tsx
Outdated
Show resolved
Hide resolved
rahul-sharma-uipath
force-pushed
the
feat/skeleton-loader-nodes
branch
from
9c3e44f to
059c408
Compare
| export const BaseSkeletonIcon = styled(ApSkeleton)<{ | ||
| shape?: NodeShape; | ||
| nodeHeight?: number; | ||
| nodeWidth?: number; | ||
| }>` |
There was a problem hiding this comment.
BaseSkeletonIcon introduces custom props (shape, nodeHeight, nodeWidth) that will be forwarded through ApSkeleton onto the underlying <div> via {...rest}, resulting in invalid DOM attributes and React warnings. Use transient props (e.g. $shape, $nodeHeight, $nodeWidth) and/or shouldForwardProp to prevent these styling-only props from reaching the DOM.
| {data.loading ? ( | ||
| <BaseSkeletonIcon | ||
| variant="rectangle" | ||
| shape={displayShape} | ||
| color={displayColor} | ||
| backgroundColor={displayIconBackground} | ||
| height={displayFooter ? undefined : height} | ||
| width={displayFooter ? undefined : (width ?? height)} | ||
| > | ||
| {Icon} | ||
| </BaseIconWrapper> | ||
| nodeHeight={displayFooter ? undefined : height} | ||
| nodeWidth={displayFooter ? undefined : (width ?? height)} | ||
| /> | ||
| ) : ( | ||
| Icon && ( | ||
| <BaseIconWrapper | ||
| shape={displayShape} | ||
| color={displayColor} | ||
| backgroundColor={displayIconBackground} | ||
| height={displayFooter ? undefined : height} | ||
| width={displayFooter ? undefined : (width ?? height)} | ||
| > | ||
| {Icon} | ||
| </BaseIconWrapper> | ||
| ) | ||
| )} |
There was a problem hiding this comment.
The new data.loading rendering path changes visible output but there are no automated tests covering the skeleton state (e.g., asserting ap-skeleton is rendered and the icon isn’t). Since this is library code under packages/, add a unit/component test for BaseNode’s loading behavior to prevent regressions.
1 participant
use
loadingattribute inNodeDatato render skeleton loader for BaseNode