Skip to content

feat: limit badges to 2 with hover/click popover in UserListTable#103

Open
tomerqodo wants to merge 2 commits into
qodo_action_req_1_base_feat_limit_badges_to_2_with_hover_click_popover_in_userlisttable_pr8from
qodo_action_req_1_head_feat_limit_badges_to_2_with_hover_click_popover_in_userlisttable_pr8
Open

feat: limit badges to 2 with hover/click popover in UserListTable#103
tomerqodo wants to merge 2 commits into
qodo_action_req_1_base_feat_limit_badges_to_2_with_hover_click_popover_in_userlisttable_pr8from
qodo_action_req_1_head_feat_limit_badges_to_2_with_hover_click_popover_in_userlisttable_pr8

Conversation

@tomerqodo

Copy link
Copy Markdown

Benchmark PR from agentic-review-benchmarks#8

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (1) 📎 Requirement gaps (0)

Grey Divider


Action required

1. Long onCheckedChange line 📘 Rule violation ⛯ Reliability
Description
onCheckedChange is written as a single very long line, exceeding the 110 character line-width
  standard required by the repository’s Biome formatting rules.
• This breaks the standardized style constraints (line width/formatting consistency) and will likely
  cause formatter/linter churn or CI failures when Biome is enforced.
Code

apps/web/modules/users/components/UserTable/UserListTable.tsx[285]

+            onCheckedChange={(value: boolean | "indeterminate") => table.toggleAllPageRowsSelected(!!value)}
Evidence
PR Compliance ID 9 requires TypeScript/JS code to follow Biome formatting rules including a 110
character line width. The new onCheckedChange callback line (and similar long lines nearby) exceed
this limit and should be wrapped/formatted.

AGENTS.md
apps/web/modules/users/components/UserTable/UserListTable.tsx[282-287]
apps/web/modules/users/components/UserTable/UserListTable.tsx[541-543]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Some newly added/edited lines exceed the Biome-enforced 110 character line width (notably `onCheckedChange` and `getFacetedUniqueValues`), violating the repo’s standardized formatting rules.

## Issue Context
The compliance checklist requires Biome formatting with a 110 character line width for TS/JS. These long lines should be wrapped to avoid CI/lint failures and reduce formatting churn.

## Fix Focus Areas
- apps/web/modules/users/components/UserTable/UserListTable.tsx[282-287]
- apps/web/modules/users/components/UserTable/UserListTable.tsx[541-543]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Hidden items not clickable 🐞 Bug ✓ Correctness
Description
• LimitedBadges only applies item.onClick to the visible badges; overflow (hidden) items are
  rendered as plain <span> elements without any click handler.
• Callers (notably the Users table Teams column) pass onClick handlers to filter the table; these
  will silently stop working whenever an item is pushed into the overflow popover.
• Because the hidden items are <span> elements, they are also not keyboard-accessible like the
  clickable Badge buttons are.
Code

apps/web/components/ui/LimitedBadges.tsx[R91-98]

+            <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>
+              ))}
Evidence
LimitedBadges renders overflow items as non-interactive spans (no onClick), even though BadgeItem
supports onClick and visible badges forward it. In UserListTable, each team item includes an onClick
that sets a column filter; overflow items therefore can’t trigger filtering. The Badge component
specifically becomes a <button> only when onClick is provided, reinforcing that spans are the wrong
primitive for interactive items.

apps/web/components/ui/LimitedBadges.tsx[66-99]
apps/web/modules/users/components/UserTable/UserListTable.tsx[370-393]
packages/ui/components/badge/Badge.tsx[55-109]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Hidden/overflow items in `LimitedBadges` are currently rendered as plain `&lt;span&gt;` elements and do **not** invoke `BadgeItem.onClick`. This breaks consumer functionality (e.g., click-to-filter in the Users table) whenever an item appears in the overflow popover.

### Issue Context
`Badge` renders as a `&lt;button&gt;` when `onClick` is provided, so using spans for interactive overflow items also reduces accessibility (keyboard interaction).

### Fix Focus Areas
- apps/web/components/ui/LimitedBadges.tsx[66-101]
- apps/web/modules/users/components/UserTable/UserListTable.tsx[385-393]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



ⓘ The new review experience is currently in Beta. Learn more

Qodo Logo

<Checkbox
checked={table.getIsAllPageRowsSelected()}
onCheckedChange={(value) => table.toggleAllPageRowsSelected(!!value)}
onCheckedChange={(value: boolean | "indeterminate") => table.toggleAllPageRowsSelected(!!value)}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Long oncheckedchange line 📘 Rule violation ⛯ Reliability

onCheckedChange is written as a single very long line, exceeding the 110 character line-width
  standard required by the repository’s Biome formatting rules.
• This breaks the standardized style constraints (line width/formatting consistency) and will likely
  cause formatter/linter churn or CI failures when Biome is enforced.
Agent prompt
## Issue description
Some newly added/edited lines exceed the Biome-enforced 110 character line width (notably `onCheckedChange` and `getFacetedUniqueValues`), violating the repo’s standardized formatting rules.

## Issue Context
The compliance checklist requires Biome formatting with a 110 character line width for TS/JS. These long lines should be wrapped to avoid CI/lint failures and reduce formatting churn.

## Fix Focus Areas
- apps/web/modules/users/components/UserTable/UserListTable.tsx[282-287]
- apps/web/modules/users/components/UserTable/UserListTable.tsx[541-543]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +91 to +98
<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>
))}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

2. Hidden items not clickable 🐞 Bug ✓ Correctness

• LimitedBadges only applies item.onClick to the visible badges; overflow (hidden) items are
  rendered as plain <span> elements without any click handler.
• Callers (notably the Users table Teams column) pass onClick handlers to filter the table; these
  will silently stop working whenever an item is pushed into the overflow popover.
• Because the hidden items are <span> elements, they are also not keyboard-accessible like the
  clickable Badge buttons are.
Agent prompt
### Issue description
Hidden/overflow items in `LimitedBadges` are currently rendered as plain `<span>` elements and do **not** invoke `BadgeItem.onClick`. This breaks consumer functionality (e.g., click-to-filter in the Users table) whenever an item appears in the overflow popover.

### Issue Context
`Badge` renders as a `<button>` when `onClick` is provided, so using spans for interactive overflow items also reduces accessibility (keyboard interaction).

### Fix Focus Areas
- apps/web/components/ui/LimitedBadges.tsx[66-101]
- apps/web/modules/users/components/UserTable/UserListTable.tsx[385-393]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant