feat: limit badges to 2 with hover/click popover in UserListTable #19
Conversation
WalkthroughIntroduces a new LimitedBadges component for compact badge rendering with overflow handling and popover tooltips. RefactorsResponseValueCell to use the new component, removes rowId prop, and enhances UserListTable with explicit TypeScript typing and React Table integration improvements while expanding its public API exports. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 3
🤖 Fix all issues with AI agents
In `@apps/web/components/ui/LimitedBadges.tsx`:
- Around line 91-98: Hidden items rendered inside the popover lose click
behavior because the span uses only item.label and never attaches the optional
onClick from the BadgeItem shape; update the render in LimitedBadges so each
hidden item forwards its onClick (e.g., use item.onClick or render the existing
BadgeItem component) and keep the cursor-pointer/hover styles, ensuring the
element uses key={item.label} (or key fallback) and calls item.onClick when
present to restore consistent clickable behavior between visible and hidden
badges.
In `@apps/web/modules/users/components/UserTable/UserListTable.tsx`:
- Around line 385-393: The LimitedBadges usage in UserListTable passes an
onClick per team to set the "teams" column filter, but overflowed/hidden items
in LimitedBadges' popover currently drop the onClick so those teams can't be
clicked to filter; update the LimitedBadges component to propagate the item's
onClick to both visible badges and to badges rendered inside the overflow
popover (preserve the same item shape: { label, variant, onClick }) and ensure
the popover renders the clickable element (or attaches the handler) so
table.getColumn("teams")?.setFilterValue([team.name]) works for overflow items
as well.
- Around line 243-253: The label construction in UserListTable (inside the
component where label, weight, groupIndicator are built) always inserts a space
before weight, causing trailing spaces when attribute.isWeightsEnabled is false;
update the logic to only add the weight segment (with its leading space or
percent) when attribute.isWeightsEnabled is true and only append the group
indicator when isAGroupOption is true — e.g., build label from
attributeValue.value and conditionally concat ` ${attributeValue.weight ||
100}%` and ` (group)` or use an array of parts [attributeValue.value,
weightPart, groupPart]. Ensure references: attribute.isWeightsEnabled,
attributeValue.weight, attributeValue.value, isAGroupOption, and the label
variable are updated accordingly.
🧹 Nitpick comments (2)
apps/web/components/ui/LimitedBadges.tsx (2)
7-7: Remove unuseduseEffectimport.The
useEffecthook is imported but never used in this component.Suggested fix
-import { useCallback, useMemo, useState, useEffect } from "react"; +import { useCallback, useMemo, useState } from "react";
68-72: Consider using a stable unique key instead ofitem.label.If two items have the same label, React will emit a key warning and may behave unexpectedly. Consider adding an
idfield toBadgeItemor using the index as a fallback key.Suggested fix using index as fallback
{visibleItems.map((item, index) => ( - <Badge key={item.label} variant={item.variant || "gray"} onClick={item.onClick}> + <Badge key={`${item.label}-${index}`} variant={item.variant || "gray"} onClick={item.onClick}> {item.label} </Badge> ))}
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/web/components/ui/LimitedBadges.tsxapps/web/modules/insights/components/ResponseValueCell.tsxapps/web/modules/insights/hooks/useInsightsColumns.tsxapps/web/modules/users/components/UserTable/UserListTable.tsx
💤 Files with no reviewable changes (1)
- apps/web/modules/insights/hooks/useInsightsColumns.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/modules/insights/components/ResponseValueCell.tsx (1)
apps/web/components/ui/LimitedBadges.tsx (1)
LimitedBadges(107-107)
🔇 Additional comments (6)
apps/web/components/ui/LimitedBadges.tsx (1)
34-105: Good component design with responsive hover behavior.The component appropriately uses
useMediaQueryto disable hover-to-open on mobile, memoizes computed values, and handles the empty state correctly.apps/web/modules/insights/components/ResponseValueCell.tsx (1)
1-24: Clean refactor usingLimitedBadges.The component is now simpler and more maintainable. Good use of
as constfor the variant literal type and the fallback toidwhen the optionMap lookup fails.apps/web/modules/users/components/UserTable/UserListTable.tsx (4)
274-297: Explicit typing for checkbox handlers improves type safety.The
onCheckedChangehandlers are now correctly typed withboolean | "indeterminate", aligning with the Checkbox component's expected signature.
518-518: Correct inclusion oftin dependency array.Including the translation function
tensures columns re-render when the locale changes.
541-574: Properly typed table configuration.The
getRowIdandgetFacetedUniqueValuesimplementations are correctly typed and handle the various column filter scenarios appropriately.
745-747: Clean export pattern.Named exports for both the component and its props type provide a clear public API.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| <div className="flex flex-col gap-1"> | ||
| {hiddenItems.map((item, index) => ( | ||
| <span | ||
| key={item.label} | ||
| className="text-default cursor-pointer text-sm hover:text-emphasis"> | ||
| {item.label} | ||
| </span> | ||
| ))} |
There was a problem hiding this comment.
Hidden items lose onClick functionality in the popover.
Items in the overflow popover have cursor-pointer styling but no onClick handler attached, even though BadgeItem supports an optional onClick. This creates inconsistent behavior where visible items are clickable but hidden items are not.
Proposed fix
<div className="flex flex-col gap-1">
{hiddenItems.map((item, index) => (
<span
- key={item.label}
- className="text-default cursor-pointer text-sm hover:text-emphasis">
+ key={`${item.label}-${index}`}
+ role={item.onClick ? "button" : undefined}
+ onClick={item.onClick}
+ className={`text-default text-sm hover:text-emphasis ${item.onClick ? "cursor-pointer" : ""}`}>
{item.label}
</span>
))}
</div>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div className="flex flex-col gap-1"> | |
| {hiddenItems.map((item, index) => ( | |
| <span | |
| key={item.label} | |
| className="text-default cursor-pointer text-sm hover:text-emphasis"> | |
| {item.label} | |
| </span> | |
| ))} | |
| <div className="flex flex-col gap-1"> | |
| {hiddenItems.map((item, index) => ( | |
| <span | |
| key={`${item.label}-${index}`} | |
| role={item.onClick ? "button" : undefined} | |
| onClick={item.onClick} | |
| className={`text-default text-sm hover:text-emphasis ${item.onClick ? "cursor-pointer" : ""}`}> | |
| {item.label} | |
| </span> | |
| ))} | |
| </div> |
🤖 Prompt for AI Agents
In `@apps/web/components/ui/LimitedBadges.tsx` around lines 91 - 98, Hidden items
rendered inside the popover lose click behavior because the span uses only
item.label and never attaches the optional onClick from the BadgeItem shape;
update the render in LimitedBadges so each hidden item forwards its onClick
(e.g., use item.onClick or render the existing BadgeItem component) and keep the
cursor-pointer/hover styles, ensuring the element uses key={item.label} (or key
fallback) and calls item.onClick when present to restore consistent clickable
behavior between visible and hidden badges.
| let weight = ""; | ||
| if (attribute.isWeightsEnabled) { | ||
| weight = `${attributeValue.weight || 100}%`; | ||
| } | ||
| let groupIndicator = ""; | ||
| if (isAGroupOption) { | ||
| groupIndicator = " (group)"; | ||
| } | ||
| let label = attributeValue.value; | ||
| label = `${label} ${weight}`; | ||
| label = `${label}${groupIndicator}`; |
There was a problem hiding this comment.
Label construction leaves trailing space when weights are disabled.
When attribute.isWeightsEnabled is false, weight is an empty string, but line 252 still concatenates " " before it, resulting in labels like "value " with a trailing space.
Proposed fix
- let weight = "";
- if (attribute.isWeightsEnabled) {
- weight = `${attributeValue.weight || 100}%`;
- }
- let groupIndicator = "";
- if (isAGroupOption) {
- groupIndicator = " (group)";
- }
- let label = attributeValue.value;
- label = `${label} ${weight}`;
- label = `${label}${groupIndicator}`;
+ const weight = attribute.isWeightsEnabled
+ ? ` ${attributeValue.weight || 100}%`
+ : "";
+ const groupIndicator = isAGroupOption ? " (group)" : "";
+ const label = `${attributeValue.value}${weight}${groupIndicator}`;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let weight = ""; | |
| if (attribute.isWeightsEnabled) { | |
| weight = `${attributeValue.weight || 100}%`; | |
| } | |
| let groupIndicator = ""; | |
| if (isAGroupOption) { | |
| groupIndicator = " (group)"; | |
| } | |
| let label = attributeValue.value; | |
| label = `${label} ${weight}`; | |
| label = `${label}${groupIndicator}`; | |
| const weight = attribute.isWeightsEnabled | |
| ? ` ${attributeValue.weight || 100}%` | |
| : ""; | |
| const groupIndicator = isAGroupOption ? " (group)" : ""; | |
| const label = `${attributeValue.value}${weight}${groupIndicator}`; |
🤖 Prompt for AI Agents
In `@apps/web/modules/users/components/UserTable/UserListTable.tsx` around lines
243 - 253, The label construction in UserListTable (inside the component where
label, weight, groupIndicator are built) always inserts a space before weight,
causing trailing spaces when attribute.isWeightsEnabled is false; update the
logic to only add the weight segment (with its leading space or percent) when
attribute.isWeightsEnabled is true and only append the group indicator when
isAGroupOption is true — e.g., build label from attributeValue.value and
conditionally concat ` ${attributeValue.weight || 100}%` and ` (group)` or use
an array of parts [attributeValue.value, weightPart, groupPart]. Ensure
references: attribute.isWeightsEnabled, attributeValue.weight,
attributeValue.value, isAGroupOption, and the label variable are updated
accordingly.
| <LimitedBadges | ||
| items={teams.map((team) => ({ | ||
| label: team.name, | ||
| variant: "gray" as const, | ||
| onClick: () => { | ||
| table.getColumn("teams")?.setFilterValue([team.name]); | ||
| }}> | ||
| {team.name} | ||
| </Badge> | ||
| ))} | ||
| }, | ||
| }))} | ||
| /> |
There was a problem hiding this comment.
Teams column LimitedBadges usage will lose filter functionality for overflow items.
The onClick handler to set column filters is correctly passed, but due to the issue in LimitedBadges where hidden items don't receive the onClick handler, teams that overflow into the popover won't be filterable by clicking.
This will be resolved once the LimitedBadges component is fixed to pass onClick to hidden items.
🤖 Prompt for AI Agents
In `@apps/web/modules/users/components/UserTable/UserListTable.tsx` around lines
385 - 393, The LimitedBadges usage in UserListTable passes an onClick per team
to set the "teams" column filter, but overflowed/hidden items in LimitedBadges'
popover currently drop the onClick so those teams can't be clicked to filter;
update the LimitedBadges component to propagate the item's onClick to both
visible badges and to badges rendered inside the overflow popover (preserve the
same item shape: { label, variant, onClick }) and ensure the popover renders the
clickable element (or attaches the handler) so
table.getColumn("teams")?.setFilterValue([team.name]) works for overflow items
as well.
Benchmark PR from qodo-benchmark#724
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.