show manual list instead of identities in group carg to avoid confusion#1760
show manual list instead of identities in group carg to avoid confusion#1760
Conversation
|
Warning Rate limit exceeded
⌛ 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. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughRenames the "Identities" label to "Manual list", ensures the wallets config is always rendered (shows "No manual list" when empty), adds tooltip and muted styling to config items, replaces resize useEffect with ResizeObserver fallback, and updates tests and a content line in the group card. Changes
Sequence Diagram(s)(Skipped — changes are UI/display, local component updates and a ResizeObserver replacement; no multi-component sequential flow requiring visualization.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
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. 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/groups/page/list/card/GroupCardConfigs.tsx (1)
235-283: Fix invalid Tailwind class token on left scroll button.
tw-border-white/is not a valid Tailwind opacity token; should betw-border-white/5to match the right scroll button.🛠️ Suggested fix
- className="tw-border-white/ tw-absolute tw-left-0 tw-top-1/2 tw-z-30 tw-inline-flex tw-h-7 tw-w-7 -tw-translate-x-3 tw-translate-y-[-50%] tw-items-center tw-justify-center tw-rounded-full tw-border tw-border-solid tw-bg-iron-800 tw-text-white tw-transition tw-duration-200 tw-ease-out focus-visible:tw-outline focus-visible:tw-outline-2 focus-visible:tw-outline-offset-2 focus-visible:tw-outline-primary-500 desktop-hover:hover:tw-bg-white/10" + className="tw-border-white/5 tw-absolute tw-left-0 tw-top-1/2 tw-z-30 tw-inline-flex tw-h-7 tw-w-7 -tw-translate-x-3 tw-translate-y-[-50%] tw-items-center tw-justify-center tw-rounded-full tw-border tw-border-solid tw-bg-iron-800 tw-text-white tw-transition tw-duration-200 tw-ease-out focus-visible:tw-outline focus-visible:tw-outline-2 focus-visible:tw-outline-offset-2 focus-visible:tw-outline-primary-500 desktop-hover:hover:tw-bg-white/10"
🤖 Fix all issues with AI agents
In `@components/groups/page/list/card/GroupCardConfigs.tsx`:
- Around line 61-63: The returned string currently always includes a leading
space before "identity" when direction is falsy; change the template to only
prepend the direction label plus a trailing space when direction is present so
that with direction = null you get "identity: X" instead of " identity: X".
Update the return expression that uses direction, directionLabels, and identity
to conditionally include `${directionLabels[direction]} ` (note the space) only
when direction is truthy.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@components/groups/page/list/card/GroupCardConfigs.tsx`:
- Line 243: The Tailwind class string in the left scroll button contains an
incomplete token `tw-border-white/` which lacks an opacity value and will be
ignored; update the className in the GroupCardConfigs component (the left scroll
button JSX element) to use a valid opacity token (e.g., change
`tw-border-white/` to `tw-border-white/5` to match the right button on line 278)
so both scroll buttons have consistent border styling.
🧹 Nitpick comments (3)
components/groups/page/list/card/GroupCardConfigs.tsx (3)
90-93: Consider simplifying the category check.The
typeof rep.category?.length === "number"check is defensive but unnecessary ifrep.categoryis always a string. A simpler truthy check would suffice:- const category = - typeof rep.category?.length === "number" && rep.category.length > 0 - ? `category: ${rep.category}` - : null; + const category = rep.category ? `category: ${rep.category}` : null;If the type can be something other than
string | null | undefined, the current approach is fine.
154-165: Consider reducing duplication withgetWalletsConfig(0).The fallback config duplicates the logic in
getWalletsConfigwhen count is 0. You could simplify:const getConfigs = (): GroupCardConfigProps[] => { if (!group) { - return [ - { - key: GroupDescriptionType.WALLETS, - value: "No manual list", - label: "Manual list", - tooltip: MANUAL_LIST_TOOLTIP, - muted: true, - }, - ]; + return [getWalletsConfig(0)]; }This keeps the empty-state logic in one place.
206-231: Cleanup removes listener unconditionally.Line 229 removes the
resizelistener even whenResizeObserverwas used. While harmless (no-op), it's cleaner to track which path was taken:♻️ Suggested improvement
useEffect(() => { checkForHiddenContent(); const container = containerRef.current; - const canObserve = - typeof ResizeObserver !== "undefined" && !!containerRef.current; - - const observer = canObserve - ? new ResizeObserver(() => checkForHiddenContent()) - : null; + const canObserve = typeof ResizeObserver !== "undefined" && !!container; + let observer: ResizeObserver | null = null; + let usedFallback = false; - if (observer && container) { + if (canObserve) { + observer = new ResizeObserver(() => checkForHiddenContent()); observer.observe(container); } else { - // Fallback: still respond to window resizes + usedFallback = true; window.addEventListener("resize", checkForHiddenContent); } return () => { - if (observer && container) { + if (observer) { observer.unobserve(container); observer.disconnect(); } - window.removeEventListener("resize", checkForHiddenContent); + if (usedFallback) { + window.removeEventListener("resize", checkForHiddenContent); + } }; }, []);Also,
!!containerRef.currenton line 211 can be simplified to!!containersince it's already captured.
|
2 participants
Summary by CodeRabbit
New Features
Style
✏️ Tip: You can customize this high-level summary in your review settings.