feat(web): redesign signals page list items#267
Merged
Conversation
Refresh action rows, chips, and list layout; add greeting header. Adds class-variance-authority for chip variants. Co-authored-by: Cursor <cursoragent@cursor.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR refactors the Signals page UI by introducing class-variance-authority for variant-driven chip styling, restructuring ActionRow components to use flex-based layouts, updating five signal action row renderers to use a new ThreadRef helper with unstyled variants, simplifying the action list layout, and adding a time-aware Greeting component that replaces LeverageReport on the Signals route. ChangesSignals UI Refactor with Variant Styling & Layout Restructuring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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
🧹 Nitpick comments (2)
apps/web/src/routes/app/_workspace/_main/signal/index.tsx (1)
53-59: 💤 Low valueConsider removing commented code.
The commented
LeverageReportblock can be removed since it's replaced by theGreetingcomponent. Git history preserves the original implementation if needed later.🧹 Cleanup suggestion
<Greeting userName={user.name} /> - {/* <LeverageReport - organizationId={currentOrg.id} - organizationCreatedAt={orgCreatedAt} - userId={user.id} - userName={user.name} - posthog={posthog ?? null} - /> */} <ActionList🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/routes/app/_workspace/_main/signal/index.tsx` around lines 53 - 59, Remove the dead commented JSX for LeverageReport: delete the commented block that references LeverageReport (including props organizationId, organizationCreatedAt, userId, userName, posthog) since Greeting replaces it; ensure no other references to LeverageReport remain in the component and run a quick compile to confirm no unused-import errors for LeverageReport remain.apps/web/src/components/chips.tsx (1)
58-58: 💤 Low valueRemove redundant nullish coalescing.
Since
disabledalready defaults tofalseat line 43, the nullish coalescing operator here is unnecessary.♻️ Suggested simplification
- disabled={disabled ?? false} + disabled={disabled}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/chips.tsx` at line 58, The prop binding uses redundant nullish coalescing: remove "?? false" from the JSX so the prop passes the existing defaulted variable directly; update the usage of disabled in the component (the variable named disabled that is already defaulted in the component's props or state) to simply be disabled={disabled} (locate the JSX in chips.tsx where disabled is set and the props default near the component's props definition).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/components/signals/greeting.tsx`:
- Line 7: The current extraction of firstName from userName can produce an empty
string for whitespace-only input, so update the logic around userName and
firstName to guard against that: compute a trimmed value (e.g., const trimmed =
userName.trim()), then set firstName to trimmed ? trimmed.split(/\s+/)[0] :
undefined (or a sensible fallback like "there"), and use that fallback when
rendering the greeting; this ensures firstName is never the empty string and
avoids the awkward "Good morning, .".
---
Nitpick comments:
In `@apps/web/src/components/chips.tsx`:
- Line 58: The prop binding uses redundant nullish coalescing: remove "?? false"
from the JSX so the prop passes the existing defaulted variable directly; update
the usage of disabled in the component (the variable named disabled that is
already defaulted in the component's props or state) to simply be
disabled={disabled} (locate the JSX in chips.tsx where disabled is set and the
props default near the component's props definition).
In `@apps/web/src/routes/app/_workspace/_main/signal/index.tsx`:
- Around line 53-59: Remove the dead commented JSX for LeverageReport: delete
the commented block that references LeverageReport (including props
organizationId, organizationCreatedAt, userId, userName, posthog) since Greeting
replaces it; ensure no other references to LeverageReport remain in the
component and run a quick compile to confirm no unused-import errors for
LeverageReport remain.
🪄 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: 7f106da6-4ede-4b15-b2ed-79ce16325915
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
apps/web/package.jsonapps/web/src/components/chips.tsxapps/web/src/components/signals/action-list.tsxapps/web/src/components/signals/action-row/action-row.tsxapps/web/src/components/signals/action-row/rows.tsxapps/web/src/components/signals/greeting.tsxapps/web/src/routes/app/_workspace/_main/signal/index.tsx
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 8 files
Confidence score: 4/5
- This PR looks safe to merge with minimal risk: both findings are moderate/low severity and appear to affect display text quality rather than core app flow.
- The most impactful issue is in
apps/web/src/components/signals/greeting.tsx, where??does not handle empty/whitespace-onlyuserName, so the greeting can render a blank or awkward first-name segment for some users. apps/web/src/components/signals/action-row/rows.tsxmay render an incomplete duplicate message ("is a duplicate of ...") when the source thread is missing because the fallback reason is skipped.- Pay close attention to
apps/web/src/components/signals/greeting.tsxandapps/web/src/components/signals/action-row/rows.tsx- user-facing text can become blank or incomplete in edge cases.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/web/src/components/signals/action-row/rows.tsx">
<violation number="1" location="apps/web/src/components/signals/action-row/rows.tsx:137">
P2: `DuplicateActionRow` now skips the fallback reason when the source thread is missing, which can render an incomplete message (`"is a duplicate of ..."`) with no source thread label.</violation>
</file>
<file name="apps/web/src/components/signals/greeting.tsx">
<violation number="1" location="apps/web/src/components/signals/greeting.tsx:7">
P2: The nullish coalescing operator (`??`) doesn't guard against empty strings — it only triggers on `null`/`undefined`. When `userName` is empty or whitespace-only, `trim().split(/\s+/)[0]` yields `""` (not `null`), so the fallback never fires and the greeting renders as "Good morning, ." with no name.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
This was referenced May 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Redesigns the Signals page list: refreshed action rows, chips, and list layout; adds a greeting header on the route.
Changes
action-rowand row variants.class-variance-authority.greetingcomponent and adjusted page composition.class-variance-authorityadded toapps/web.Test plan
Made with Cursor
Summary by cubic
Redesigned the Signals list with a cleaner row layout and improved chips, plus a time-based greeting at the top. The greeting now handles empty names and uses better time ranges.
New Features
Card/Separatorwrappers in favor of a simple vertical layout with spacing; refreshed attention count text.ThreadChipnow supports avariantprop (chip|unstyled) anddisabled, with size adapting in unstyled mode; built with CVA viaclass-variance-authority.Bug Fixes
Written for commit 434865f. Summary will update on new commits.
Summary by CodeRabbit
New Features
UI Improvements