Enhancement: Add empty state for client list when no data is available#103
Enhancement: Add empty state for client list when no data is available#103heymanasvi wants to merge 1 commit into
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 13 minutes and 6 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (18)
📝 WalkthroughWalkthroughTable rendering in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/pages/clients/Clients.tsx`:
- Around line 232-235: The TableRow rows are only mouse-clickable via the
onClick handler (TableRow with onClick={() => c.id &&
navigate(`/clients/${c.id}/general`)}), so add keyboard accessibility by making
the row focusable (add tabIndex={0}), give it an appropriate role (e.g.,
role="button" or use an anchor if preferred), and implement an onKeyDown handler
that listens for Enter and Space to call the same navigate(c.id) logic (reuse
c.id && navigate(`/clients/${c.id}/general`)). Ensure you keep the existing
onClick behavior and preserve aria/visual focus styles for accessibility.
- Around line 221-231: The current UI uses filtered.length === 0 to show a
single hardcoded English message, causing “No clients available” to appear even
when there are clients but filters exclude them and also bypassing localization;
update the render logic to check the unfiltered source (e.g., the full clients
array/state) versus filtered: if the full clients collection is empty render a
localized "no clients available" message (use the app's i18n helper, e.g.,
t('clients.noClients')), else if full is non-empty but filtered.length === 0
render a localized "no clients match your filters" message (e.g.,
t('clients.noMatches')) inside the existing TableRow/TableCell; keep ClientRow
mapping unchanged for the non-empty case.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 61e9b57f-37ca-4b7f-a66d-27228c572f0c
📒 Files selected for processing (1)
src/pages/clients/Clients.tsx
gkbishnoi07
left a comment
gkbishnoi07
left a comment
There was a problem hiding this comment.
please create jira ticket or if already created then mention it
|
Hi @gkbishnoi07. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/pages/clients/Clients.tsx (1)
241-253:⚠️ Potential issue | 🟡 MinorAvoid focusable/button rows when navigation is unavailable (
c.idmissing).At Line 243-Line 245, rows are always exposed as interactive, but handlers are no-ops when
c.idis absent. This creates dead keyboard targets and weakens key stability.♿ Suggested fix
- filtered.map((c: ClientRow) => ( + filtered.map((c: ClientRow, index: number) => { + const canNavigate = Boolean(c.id) + const goToClient = () => + canNavigate && navigate(`/clients/${c.id}/general`) + + return ( <TableRow - key={c.id} - tabIndex={0} - role="button" - onClick={() => c.id && navigate(`/clients/${c.id}/general`)} + key={c.id ?? `client-row-${index}`} + tabIndex={canNavigate ? 0 : -1} + role={canNavigate ? 'button' : undefined} + onClick={goToClient} onKeyDown={e => { + if (!canNavigate) return if (e.key === 'Enter' || e.key === ' ') { e.preventDefault() - c.id && navigate(`/clients/${c.id}/general`) + goToClient() } }} - className="cursor-pointer hover:bg-zinc-100 dark:hover:bg-zinc-700 transition-colors text-base" + className={`${ + canNavigate ? 'cursor-pointer' : '' + } hover:bg-zinc-100 dark:hover:bg-zinc-700 transition-colors text-base`} > @@ - </TableRow> - )) + </TableRow> + )})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/clients/Clients.tsx` around lines 241 - 253, The TableRow currently renders as an interactive element even when c.id is falsy, creating dead keyboard targets; update the TableRow rendering so that when c.id is present you keep tabIndex={0}, role="button", onClick and onKeyDown handlers, and the "cursor-pointer" hover classes (the interactive state that calls navigate(`/clients/${c.id}/general`)), but when c.id is missing render a non-interactive row (no tabIndex, no role, no onClick/onKeyDown, and remove pointer/hover classes) or explicitly render a separate non-interactive variant; adjust the conditional around the TableRow props (referencing TableRow, c.id, navigate, onClick, onKeyDown) so keyboard users only focus rows that will actually navigate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/pages/clients/Clients.tsx`:
- Around line 227-236: The component Clients.tsx uses i18n keys 'empty' and
'noMatch' but only en-US defines them; update every locale JSON (all locale
variant files) to include these two keys with appropriate translated strings for
'empty' and 'noMatch' so non-English users don't see missing-key fallbacks—add
entries named "empty" and "noMatch" to each locale resource file alongside
existing clients-related keys.
---
Duplicate comments:
In `@src/pages/clients/Clients.tsx`:
- Around line 241-253: The TableRow currently renders as an interactive element
even when c.id is falsy, creating dead keyboard targets; update the TableRow
rendering so that when c.id is present you keep tabIndex={0}, role="button",
onClick and onKeyDown handlers, and the "cursor-pointer" hover classes (the
interactive state that calls navigate(`/clients/${c.id}/general`)), but when
c.id is missing render a non-interactive row (no tabIndex, no role, no
onClick/onKeyDown, and remove pointer/hover classes) or explicitly render a
separate non-interactive variant; adjust the conditional around the TableRow
props (referencing TableRow, c.id, navigate, onClick, onKeyDown) so keyboard
users only focus rows that will actually navigate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 66729e53-1b31-479c-b560-035f363123cc
📒 Files selected for processing (2)
src/locales/en-US/clients.jsonsrc/pages/clients/Clients.tsx
✅ Files skipped from review due to trivial changes (1)
- src/locales/en-US/clients.json
|
Hii @gkbishnoi07
|
gkbishnoi07
left a comment
gkbishnoi07
left a comment
There was a problem hiding this comment.
can you please squash your commits make it 1
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/pages/clients/Clients.tsx (1)
220-238: Empty/no-match branches look correct, but pagination caption can mislead in the empty state.The three-branch rendering correctly distinguishes an empty dataset from filter-excluded rows, and uses i18n keys that are now defined across all locales. One adjacent nit:
TableCaptionat Lines 195-202 still showspagination.showingwithtotalandpageseven whenrows.length === 0, which renders something like "Showing 0 of 0, page 1 of 1" above the empty-state message. Consider hiding the caption (or the pagination controls entirely) whenrows.length === 0to avoid contradicting the empty state.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/clients/Clients.tsx` around lines 220 - 238, The TableCaption/pagination caption (rendered near TableCaption) can mislead when there are no rows; update the Clients component to only render the TableCaption (and any pagination controls) when rows.length > 0. Locate the TableCaption usage and wrap its rendering in a conditional check (e.g., rows.length > 0) so it is omitted for the empty dataset while keeping the three-branch TableBody logic (rows.length === 0 / filtered.length === 0) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/pages/clients/Clients.tsx`:
- Around line 241-258: Restore the missing interactive styling and prevent
non-navigable rows from being focusable: on the TableRow that uses c.id, re-add
the previous className (e.g., "cursor-pointer hover:bg-zinc-100
dark:hover:bg-zinc-700 transition-colors text-base") to restore pointer, hover
and text sizing, and wrap or conditionalize role="button" and tabIndex={0} so
they are only present when c.id is truthy (the same condition used in the
onClick and onKeyDown handlers that call navigate). Also add a keyboard focus
affordance (e.g., a focus-visible ring) in the same className so keyboard users
see focus, and keep the existing onClick/onKeyDown logic unchanged except for
the conditional role/tabIndex gating.
---
Nitpick comments:
In `@src/pages/clients/Clients.tsx`:
- Around line 220-238: The TableCaption/pagination caption (rendered near
TableCaption) can mislead when there are no rows; update the Clients component
to only render the TableCaption (and any pagination controls) when rows.length >
0. Locate the TableCaption usage and wrap its rendering in a conditional check
(e.g., rows.length > 0) so it is omitted for the empty dataset while keeping the
three-branch TableBody logic (rows.length === 0 / filtered.length === 0)
unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7aef68c9-9407-4b24-92d5-8ffcb50b945b
📒 Files selected for processing (2)
client.jsonsrc/pages/clients/Clients.tsx
✅ Files skipped from review due to trivial changes (1)
- client.json
| <TableRow | ||
| key={c.id} | ||
| tabIndex={0} | ||
| role="button" | ||
| onClick={() => { | ||
| if (c.id) { | ||
| navigate(`/clients/${c.id}/general`) | ||
| } | ||
| }} | ||
| onKeyDown={e => { | ||
| if (e.key === 'Enter' || e.key === ' ') { | ||
| e.preventDefault() | ||
| if (c.id) { | ||
| navigate(`/clients/${c.id}/general`) | ||
| } | ||
| } | ||
| }} | ||
| > |
There was a problem hiding this comment.
Row styling regression and unconditional role="button"/tabIndex on non-navigable rows.
Two concerns on the refactored TableRow:
- The previous
className="cursor-pointer hover:bg-zinc-100 dark:hover:bg-zinc-700 transition-colors text-base"was dropped. Rows now lack the pointer cursor, hover affordance, and consistent text sizing — a visible UX regression versus the prior behavior. tabIndex={0}androle="button"are applied unconditionally, but both click and keydown handlers no-op whenc.idis missing. Focusable "button" rows that do nothing are misleading to keyboard/AT users. Gate them onc.id, and consider afocus-visiblering for keyboard users.
♻️ Proposed fix
filtered.map((c: ClientRow) => (
<TableRow
key={c.id}
- tabIndex={0}
- role="button"
+ tabIndex={c.id ? 0 : -1}
+ role={c.id ? 'button' : undefined}
+ className="cursor-pointer hover:bg-zinc-100 dark:hover:bg-zinc-700 transition-colors text-base focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[`#1074b9`]"
onClick={() => {
if (c.id) {
navigate(`/clients/${c.id}/general`)
}
}}
onKeyDown={e => {
if (e.key === 'Enter' || e.key === ' ') {
e.preventDefault()
if (c.id) {
navigate(`/clients/${c.id}/general`)
}
}
}}
>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/clients/Clients.tsx` around lines 241 - 258, Restore the missing
interactive styling and prevent non-navigable rows from being focusable: on the
TableRow that uses c.id, re-add the previous className (e.g., "cursor-pointer
hover:bg-zinc-100 dark:hover:bg-zinc-700 transition-colors text-base") to
restore pointer, hover and text sizing, and wrap or conditionalize role="button"
and tabIndex={0} so they are only present when c.id is truthy (the same
condition used in the onClick and onKeyDown handlers that call navigate). Also
add a keyboard focus affordance (e.g., a focus-visible ring) in the same
className so keyboard users see focus, and keep the existing onClick/onKeyDown
logic unchanged except for the conditional role/tabIndex gating.
heymanasvi
force-pushed
the
fix-empty-state-clients
branch
from
8f09f5e to
77c03dc
Compare
|
Hi @gkbishnoi07 , just a gentle follow-up on this PR. Whenever you get time, could you please review it? Thanks! |
2 participants
Description
This change adds a simple empty state to the clients page when no data is available.
Earlier, when the client list was empty, the table showed no rows, which could be confusing. With this update, a clear message is displayed to indicate that no clients are available.
This improves the user experience without affecting existing functionality.
##Related issues
Jira Issue: WEB-911
Link: https://mifosforge.jira.com/browse/WEB-911
Summary by CodeRabbit