Consumer side-by-side shell, row restyle, and expand/collapse for N-row question types#4712
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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 |
Cleanup: Preview Environment RemovedThe preview environment for this PR has been destroyed.
Cleanup triggered by PR close at 2026-05-15T14:08:01Z |
ad9b7c1 to
89df406
Compare
bd13d85 to
60d3576
Compare
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)
front_end/src/components/consumer_post_card/group_forecast_card/percentage_forecast_card.tsx (1)
1-42:⚠️ Potential issue | 🟠 Major | ⚡ Quick winContext hook requires explicit provider or fallback handling to work correctly.
Lines 6 and 42 use
useListChartExpanded()unconditionally. While this won't throw a runtime error (the context has default no-op values), it creates a silent bug:setIsExpandedbecomes a no-op function in contexts that don't provideListChartExpandedContext.PercentageForecastCard is already used in at least 3 locations without the
ConsumerListChartShellprovider:
group_of_questions_prediction/index.tsxlabor-hub/components/question_cards/basic_question.tsxsidebar/similar_questions/similar_question_prediction_chip.tsxIn these contexts, the card's expand/collapse state won't propagate to the parent, breaking any parent-level UI coordination that depends on
setIsExpanded.Either wrap all usage sites in
ConsumerListChartShell, or add proper fallback handling to make the hook optional:- const { setIsExpanded } = useListChartExpanded(); + const context = useListChartExpanded(); + const setIsExpanded = context?.setIsExpanded ?? (() => {});🤖 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 `@front_end/src/components/consumer_post_card/group_forecast_card/percentage_forecast_card.tsx` around lines 1 - 42, The component calls useListChartExpanded() and always invokes setIsExpanded, which silently no-ops when the provider is missing; update PercentageForecastCard to handle the hook being optional by checking the hook return before using it (e.g., const ctx = useListChartExpanded(); if (ctx?.setIsExpanded) ctx.setIsExpanded(true/false) or guard any handlers that call setIsExpanded), so all usages of setIsExpanded are conditional and safe when ConsumerListChartShell is not present; ensure only the functions/handlers in PercentageForecastCard that currently call setIsExpanded reference ctx?.setIsExpanded rather than assuming it exists.
♻️ Duplicate comments (1)
front_end/src/components/consumer_post_card/group_forecast_card/numeric_forecast_card.tsx (1)
1-42:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winSame context dependency issue as PercentageForecastCard.
Line 41 unconditionally calls
useListChartExpanded(), which will throw ifNumericForecastCardis used outside aConsumerListChartShellprovider. See the earlier comment onPercentageForecastCardfor details and verification steps.🤖 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 `@front_end/src/components/consumer_post_card/group_forecast_card/numeric_forecast_card.tsx` around lines 1 - 42, NumericForecastCard unconditionally calls the context hook useListChartExpanded (and uses setIsExpanded/setHoveredChoiceName) which will throw when rendered outside the ConsumerListChartShell provider; fix by making the hook optional: call useListChartExpanded safely (e.g., allow it to return undefined/null) or wrap its usage in a guard so NumericForecastCard checks for a non-null context before calling setIsExpanded or setHoveredChoiceName, and update any event handlers in NumericForecastCard to no-op when the context is absent so the component can be used both inside and outside the ConsumerListChartShell provider.
🧹 Nitpick comments (2)
front_end/src/components/consumer_post_card/group_forecast_card/percentage_forecast_card.tsx (1)
132-147: ⚖️ Poor tradeoffHardcoded absolute positioning assumes specific parent layout.
The expanded overlay (line 133) uses hardcoded offsets
-left-[21px] -top-[21px]andw-[calc(100%+42px)], which assumes the parent container has exactly 21px of padding/spacing. If the parent layout changes or this component is used in a different context, the overlay will be misaligned.Consider:
- Using CSS variables for offset values
- Computing position dynamically based on parent dimensions
- Using a portal with absolute positioning relative to viewport
🤖 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 `@front_end/src/components/consumer_post_card/group_forecast_card/percentage_forecast_card.tsx` around lines 132 - 147, The expansion overlay in percentage_forecast_card.tsx is using hardcoded offsets (-left-[21px], -top-[21px], w-[calc(100%+42px)]) which assumes a specific parent padding; change the positioning to be layout-agnostic by removing fixed pixel offsets on the wrapping div and instead use a CSS variable (e.g., --card-offset) or inset-based utilities (e.g., inset-0 with padding adjustments) so the overlay expands to the parent's bounds; update the component wrapping the content (the div that contains ForecastCardWrapper and the expanded conditional) to read the CSS var or compute offset from the parent (or render the expanded panel into a portal positioned relative to viewport if that suits your use-case) and ensure state handlers (setExpanded, setIsExpanded) and renderBars(allChoices) remain unchanged.front_end/src/app/(main)/questions/[id]/components/question_view/consumer_question_view/consumer_list_chart_shell.tsx (1)
40-43: 💤 Low valueConsider simplifying the useMemo dependency array.
React guarantees that
setStatefunctions (likesetIsExpandedandsetHoveredChoiceName) are stable and never change between renders. Including them in the dependency array is redundant.♻️ Simplified dependency array
const contextValue = useMemo( () => ({ setIsExpanded, hoveredChoiceName, setHoveredChoiceName }), - [hoveredChoiceName, setHoveredChoiceName, setIsExpanded] + [hoveredChoiceName] );🤖 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 `@front_end/src/app/`(main)/questions/[id]/components/question_view/consumer_question_view/consumer_list_chart_shell.tsx around lines 40 - 43, The useMemo call creating contextValue currently lists stable setState functions in its dependency array; simplify it by removing setIsExpanded and setHoveredChoiceName and only include hoveredChoiceName as the dependency. Update the useMemo invocation (useMemo(() => ({ setIsExpanded, hoveredChoiceName, setHoveredChoiceName }), [hoveredChoiceName])) so React relies on the stable setter identities and only re-computes when hoveredChoiceName changes.
🤖 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
`@front_end/src/components/consumer_post_card/group_forecast_card/forecast_card_wrapper.tsx`:
- Around line 56-71: The expand button currently renders as a button with
onClick={isMinimal ? undefined : onExpand}, producing a non-interactive button
when isMinimal is true; update the render logic for the element used in
ForecastCardWrapper so it either becomes explicitly disabled or is not a button:
when isMinimal is true set disabled={true} and aria-disabled="true" and add
disabled styling to toggleButtonClassName (or alternatively render a non-button
element like a div/span for the minimal variant), ensuring the FontAwesomeIcon
and otherItemsCount remain visible and that the onExpand handler is only
attached for the interactive variant to restore accessibility parity with the
collapse control.
---
Outside diff comments:
In
`@front_end/src/components/consumer_post_card/group_forecast_card/percentage_forecast_card.tsx`:
- Around line 1-42: The component calls useListChartExpanded() and always
invokes setIsExpanded, which silently no-ops when the provider is missing;
update PercentageForecastCard to handle the hook being optional by checking the
hook return before using it (e.g., const ctx = useListChartExpanded(); if
(ctx?.setIsExpanded) ctx.setIsExpanded(true/false) or guard any handlers that
call setIsExpanded), so all usages of setIsExpanded are conditional and safe
when ConsumerListChartShell is not present; ensure only the functions/handlers
in PercentageForecastCard that currently call setIsExpanded reference
ctx?.setIsExpanded rather than assuming it exists.
---
Duplicate comments:
In
`@front_end/src/components/consumer_post_card/group_forecast_card/numeric_forecast_card.tsx`:
- Around line 1-42: NumericForecastCard unconditionally calls the context hook
useListChartExpanded (and uses setIsExpanded/setHoveredChoiceName) which will
throw when rendered outside the ConsumerListChartShell provider; fix by making
the hook optional: call useListChartExpanded safely (e.g., allow it to return
undefined/null) or wrap its usage in a guard so NumericForecastCard checks for a
non-null context before calling setIsExpanded or setHoveredChoiceName, and
update any event handlers in NumericForecastCard to no-op when the context is
absent so the component can be used both inside and outside the
ConsumerListChartShell provider.
---
Nitpick comments:
In
`@front_end/src/app/`(main)/questions/[id]/components/question_view/consumer_question_view/consumer_list_chart_shell.tsx:
- Around line 40-43: The useMemo call creating contextValue currently lists
stable setState functions in its dependency array; simplify it by removing
setIsExpanded and setHoveredChoiceName and only include hoveredChoiceName as the
dependency. Update the useMemo invocation (useMemo(() => ({ setIsExpanded,
hoveredChoiceName, setHoveredChoiceName }), [hoveredChoiceName])) so React
relies on the stable setter identities and only re-computes when
hoveredChoiceName changes.
In
`@front_end/src/components/consumer_post_card/group_forecast_card/percentage_forecast_card.tsx`:
- Around line 132-147: The expansion overlay in percentage_forecast_card.tsx is
using hardcoded offsets (-left-[21px], -top-[21px], w-[calc(100%+42px)]) which
assumes a specific parent padding; change the positioning to be layout-agnostic
by removing fixed pixel offsets on the wrapping div and instead use a CSS
variable (e.g., --card-offset) or inset-based utilities (e.g., inset-0 with
padding adjustments) so the overlay expands to the parent's bounds; update the
component wrapping the content (the div that contains ForecastCardWrapper and
the expanded conditional) to read the CSS var or compute offset from the parent
(or render the expanded panel into a portal positioned relative to viewport if
that suits your use-case) and ensure state handlers (setExpanded, setIsExpanded)
and renderBars(allChoices) remain 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d55f3678-89c5-4d1b-8007-f3974e69ad6c
📒 Files selected for processing (19)
front_end/src/app/(main)/questions/[id]/components/group_timeline.tsxfront_end/src/app/(main)/questions/[id]/components/multiple_choices_chart_view/index.tsxfront_end/src/app/(main)/questions/[id]/components/question_page_shell/index.tsxfront_end/src/app/(main)/questions/[id]/components/question_view/consumer_question_view/consumer_group_chart.tsxfront_end/src/app/(main)/questions/[id]/components/question_view/consumer_question_view/consumer_list_chart_shell.tsxfront_end/src/app/(main)/questions/[id]/components/question_view/consumer_question_view/prediction/group_of_questions_prediction/index.tsxfront_end/src/app/(main)/questions/[id]/components/question_view/consumer_question_view/prediction/index.tsxfront_end/src/app/(main)/questions/[id]/components/question_view/consumer_question_view/timeline/index.tsxfront_end/src/app/(main)/questions/[id]/components/sidebar/similar_questions/similar_question_prediction_chip.tsxfront_end/src/components/charts/group_chart.tsxfront_end/src/components/consumer_post_card/group_forecast_card/forecast_card_wrapper.tsxfront_end/src/components/consumer_post_card/group_forecast_card/forecast_choice_bar.tsxfront_end/src/components/consumer_post_card/group_forecast_card/index.tsxfront_end/src/components/consumer_post_card/group_forecast_card/numeric_forecast_card.tsxfront_end/src/components/consumer_post_card/group_forecast_card/percentage_forecast_card.tsxfront_end/src/components/consumer_post_card/index.tsxfront_end/src/components/detailed_question_card/detailed_group_card/index.tsxfront_end/src/components/detailed_question_card/detailed_question_card/index.tsxfront_end/src/components/detailed_question_card/detailed_question_card/multiple_choice_chart_card.tsx
df30316 to
3c52441
Compare
3d8870e to
468ddad
Compare
…de-by-side shell and fix styles in ConsumerListChartShell
…/collapse and expanded panel
…hide legend in consumer MC and binary-group chart views
…Chart in fan graph left panel
…estions with proportional bars, hover-synced chart highlight, and endpoint dots
…ar-row types, cover discrete groups, and prevent expand height jump
468ddad to
8db0a97
Compare
c39da3b
into
feat/question-page-redesign-3rd-iteration
Related to #4643
Summary
This PR introduces
ConsumerListChartShell, a shared layout primitive for consumer N-row body types, extending the side-by-side layout from iter 2 (single binary/continuous) to all four group-style body types. It also restyls the expand/collapse affordance across forecast cards with a newbuttonVariantprop, introduces a minimal ellipsis variant for the sidebar, and hides the legend bar in consumer chart views. For continuous numeric group questions, the left pane now renders proportional colored bars that vertically scale to fill the panel height (applied to all N-row bar types), with a mountedGroupTimelineon the right. Hovering a bar row isolates that subquestion's line on the chart - others dim, the highlighted line gets a heavier stroke, and endpoint dots mark the latest forecast value.Consumer side-by-side shell for N-row bodies (MC / binary group / continuous group / time series)
PercentageForecastCard, right:QuestionTimeline → DetailedMultipleChoiceChartCardPercentageForecastCard, right:QuestionTimeline → GroupTimelineNumericForecastCard, right:QuestionTimeline → GroupTimelineTimeSeriesChart(colorful vertical bars), right:DetailedGroupCard(fan chart)RevealCPButtonreplaces the left slot for all four body types when CP is hiddenRestyle colored-bar row + list-level expand/collapse (MC + binary group consumer)
buttonVariantprop ("primary"|"minimal") — minimal renders an ellipsis icon with a gray border for sidebar usebuttonVariant="minimal"so the expand affordance uses the three-dot style instead of the primary blue chevronwithLegend={false}throughDetailedGroupCard → GroupTimeline(binary group) andDetailedMultipleChoiceChartCard(MC) so the legend bar is suppressed in consumer view onlyContinuous group consumer side-by-side: row restyle + mount combined timeline + hover-sync
hoveredChoiceNamecontext state andstretchListContentprop (enables flex column on left panel so bars fill available height)onMouseEnter/onMouseLeave/classNameprops; fill height; fill fades up on hover; unfilled area gets a blue tint only on the hovered rowfillHeightprop threadsflex-1through wrapper and bars so fewer items scale up; wires per-row mouse events tohoveredChoiceNamecontexthoveredChoiceNamefrom context, rendersGroupTimelinewithexternalHighlightedChoice,withLegend={false},withHighlightArea={false},withHighlightEndpointexternalHighlightedChoiceprop syncs intochoiceItems.highlightedviauseEffect; threadedwithHighlightArea/withHighlightEndpointtoMultiChoicesChartViewonMouseLeaveresets cursor so lines don't freezeisContinuousNumericGrouptoNumericForecastCard fillHeight(left) +ConsumerGroupChart(right)Demo videos
New consumer side-by-side layout (MC / binary group / continuous group / time series)
charts.mp4
Expandable Modal Behavior
expand-modal.mp4
Proportional Bars + Hover-Synced Timeline (Continuous group consumer)
hover-count-group-1.mp4
Summary by CodeRabbit
Release Notes
New Features
Refactor