fix: loading indicator ui for infinite loading table #39208
Conversation
WalkthroughThis pull request adds a new constant Changes
Sequence Diagram(s)sequenceDiagram
participant Test
participant LI as LoadingIndicator
participant CG as createMessage (using TABLE_LOADING_RECORDS)
participant UI as UI Elements (Flex/Spinner/Text)
Test->>LI: Render LoadingIndicator component
LI->>CG: Call TABLE_LOADING_RECORDS()
CG-->>LI: Return "loading records"
LI->>UI: Render Flex container with Spinner and Text ("loading records")
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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
Documentation and Community
|
AmanAgarwal041
force-pushed
the
fix/loading-indicator-ui
branch
from
19eb088 to
8d0909a
Compare
AmanAgarwal041
requested review from
ApekshaBhosale,
sagar-qa007,
sharat87,
pratapaprasanna,
ayushpahwa,
a team and
KelvinOm
as code owners
AmanAgarwal041
requested review from
jacquesikot
and removed request for
a team
AmanAgarwal041
changed the base branch from
feat/-infinite-scroll-poc-infinite-loader
to
release
github-actions
bot
added
Bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
app/client/src/widgets/TableWidgetV2/component/LoadingIndicator.test.tsx (3)
11-13: Consider using Testing Library queries instead of querySelector.Replace
querySelectorwith Testing Library's built-in queries for more resilient tests. This aligns better with Testing Library's philosophy of testing user interactions.- const flexContainer = document.querySelector(".ads-v2-flex"); - - expect(flexContainer).toBeInTheDocument(); + const flexContainer = screen.getByTestId("loading-indicator-container"); + expect(flexContainer).toBeInTheDocument(); + expect(flexContainer).toHaveClass("ads-v2-flex");
16-18: Use Testing Library queries for spinner element.Similar to the previous comment, prefer Testing Library queries over direct DOM queries.
- const spinner = document.querySelector(".ads-v2-spinner"); - - expect(spinner).toBeInTheDocument(); + const spinner = screen.getByTestId("loading-spinner"); + expect(spinner).toBeInTheDocument(); + expect(spinner).toHaveClass("ads-v2-spinner");
6-26: Enhance test coverage with additional test cases.Consider adding tests for:
- Accessibility (role, aria-label)
- Different loading states if applicable
- Component unmounting
Example addition:
it("meets accessibility requirements", () => { render(<LoadingIndicator />); const spinner = screen.getByRole("progressbar"); expect(spinner).toHaveAttribute("aria-label", "Loading records"); });app/client/src/widgets/TableWidgetV2/component/LoadingIndicator.tsx (1)
5-22: LGTM! Consider adding prop types for future extensibility.The implementation is clean and follows best practices. It properly uses design system components and CSS variables.
Consider adding TypeScript interface for future props:
-export const LoadingIndicator = () => ( +interface LoadingIndicatorProps { + className?: string; +} + +export const LoadingIndicator = ({ className }: LoadingIndicatorProps = {}) => ( <Flex + className={className} alignItems="center" // ... rest of the props >
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/client/src/ce/constants/messages.ts(1 hunks)app/client/src/widgets/TableWidgetV2/component/LoadingIndicator.test.tsx(1 hunks)app/client/src/widgets/TableWidgetV2/component/LoadingIndicator.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- app/client/src/ce/constants/messages.ts
AmanAgarwal041
removed request for
ApekshaBhosale,
sagar-qa007,
sharat87,
pratapaprasanna,
ayushpahwa and
a team
AmanAgarwal041
requested review from
rahulbarwal
and removed request for
a team and
KelvinOm
…ading-indicator-ui
Description
Fixing the loading indicator for the infinite scroll of table widget.
Figma : https://www.figma.com/design/cBdxf04BPf5XiBCDtlMdxE/Feature-requests?node-id=20-9718&t=nhblcqfbZ2I0vxny-4
Fixes #39081
or
Fixes
Issue URLWarning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
Automation
/ok-to-test tags="@tag.Widget"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/13302156383
Commit: fb6f61b
Cypress dashboard.
Tags:
@tag.WidgetSpec:
Thu, 13 Feb 2025 08:09:36 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Tests