Open
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR refactors multiple-choice option visibility and ordering logic by introducing a dynamic visibility function ( Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
Contributor
🚀 Preview EnvironmentYour preview environment is ready!
Details
ℹ️ Preview Environment InfoIsolation:
Limitations:
Cleanup:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
front_end/src/components/post_card/multiple_choice_tile/choice_option.tsx (1)
101-109: Keep the full option text discoverable.Now that every label is single-line truncated, long options with the same prefix can collapse to the same visible text. Adding a
titlehere keeps the full wording available without changing layout.📝 Suggested tweak
<div + title={choice} className={cn( "resize-label min-w-0 flex-1 pr-2.5 text-left text-sm font-normal leading-4", "truncate", labelClassName )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/components/post_card/multiple_choice_tile/choice_option.tsx` around lines 101 - 109, The label div that renders the option text (in choice_option.tsx) is truncated and should expose the full text via a title attribute; update the div that uses cn(...) and renders {choice} to include title={choice} (or title={`${choice}`} to handle non-strings) so long options remain discoverable while keeping the single-line layout; reference the rendered element that currently uses className={cn(...)} and the choice and labelClassName props when making this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@front_end/src/app/`(main)/questions/[id]/components/multiple_choices_chart_view/choices_legend/index.tsx:
- Around line 85-89: The legend popover/dropdown still renders full labels for
hidden choices; update the popover item rendering to use the same truncation as
the pinned chips by passing truncateLabel(label || choice, 30) instead of label
|| choice. Locate the popover/dropdown mapping where hidden legend items are
rendered (the code near legendChoices.map / ChoiceCheckbox in
choices_legend/index.tsx) and replace the raw label with truncateLabel(...),
ensuring any component props or keys that display text (e.g., ChoiceCheckbox or
the dropdown item renderer) receive the truncated string.
In
`@front_end/src/components/detailed_question_card/detailed_question_card/multiple_choice_chart_card.tsx`:
- Around line 253-257: The embed path currently uses
buildChoicesWithOthers(choiceItems) which inserts a hardcoded English "Others"
label causing the legend to remain untranslated; update the embed branch in
embedChoiceItems to pass the localized label (reuse the same i18n key/translator
used by the tooltip path) into buildChoicesWithOthers or map the returned
"Others" item label through the component's translator before passing to
MultipleChoiceTile so the legend uses the translated string; reference the
embedChoiceItems constant, buildChoicesWithOthers, and MultipleChoiceTile when
making this change.
In `@front_end/src/utils/questions/choices.ts`:
- Around line 46-66: The code incorrectly assumes all inactive entries share the
same aggregationTimestamps/userTimestamps by using inactive[0] (aggTs, userTs)
and summing aggregationValues[i]/userValues[i]; instead build a unified
timestamp set from all inactive[].aggregationTimestamps and all
inactive[].userTimestamps, create per-entry maps from timestamp->value, and then
compute aggregationValues and userValues by iterating that unified timestamp
list and summing values from each map (using null for missing entries) so
timestamps are aligned across different histories; update the logic that uses
aggTs/userTs, sumNullable, aggregationValues and userValues accordingly and keep
generateChoiceItemsFromGroupQuestions()’s timestamp semantics consistent.
- Around line 201-214: The current isCPHidden branch is mutating item.active to
make the first N definition-order items active, which swaps the selected set;
instead, when isCPHidden (cpRevealsOn or hideCP logic) do not change any
item.active flags — preserve the existing CP-based selected set — and only
adjust presentation order/visible slice if needed; so in the isCPHidden branch
remove the choiceItems.forEach mutation that sets item.active, keep computing
effectiveActiveCount via getEffectiveVisibleCount for presentation purposes, and
return choiceItems (or a reordered/trimmed view) without altering any
item.active properties.
---
Nitpick comments:
In `@front_end/src/components/post_card/multiple_choice_tile/choice_option.tsx`:
- Around line 101-109: The label div that renders the option text (in
choice_option.tsx) is truncated and should expose the full text via a title
attribute; update the div that uses cn(...) and renders {choice} to include
title={choice} (or title={`${choice}`} to handle non-strings) so long options
remain discoverable while keeping the single-line layout; reference the rendered
element that currently uses className={cn(...)} and the choice and
labelClassName props when making this change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e12e446a-3f7e-4fd3-98d2-235ee8440ff4
📒 Files selected for processing (15)
front_end/src/app/(main)/questions/[id]/components/group_timeline.tsxfront_end/src/app/(main)/questions/[id]/components/multiple_choices_chart_view/choices_legend/index.tsxfront_end/src/app/(main)/questions/[id]/components/multiple_choices_chart_view/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/detailed_question_card/detailed_group_card/index.tsxfront_end/src/components/detailed_question_card/detailed_question_card/multiple_choice_chart_card.tsxfront_end/src/components/detailed_question_card/embeds.tsfront_end/src/components/forecast_maker/forecast_maker_question/forecast_maker_multiple_choice.tsxfront_end/src/components/post_card/group_of_questions_tile/index.tsxfront_end/src/components/post_card/multiple_choice_tile/choice_option.tsxfront_end/src/components/post_card/multiple_choice_tile/index.tsxfront_end/src/components/post_card/question_tile/index.tsxfront_end/src/constants/questions.tsfront_end/src/utils/questions/choices.ts
💤 Files with no reviewable changes (2)
- front_end/src/components/detailed_question_card/embeds.ts
- front_end/src/components/forecast_maker/forecast_maker_question/forecast_maker_multiple_choice.tsx
Comment on lines
85
to
89
| {legendChoices.map(({ label, choice, color, active }, idx) => ( | ||
| <ChoiceCheckbox | ||
| key={`multiple-choice-legend-${choice}-${idx}`} | ||
| label={label || choice} | ||
| label={truncateLabel(label || choice, 30)} | ||
| color={color.DEFAULT} |
Contributor
There was a problem hiding this comment.
Truncate the dropdown labels too.
Line 88 fixes the pinned legend chips, but hidden choices still render label || choice in the popover. Long MC labels can still blow out the dropdown width, so the truncation part of this change is only half-done.
Suggested follow-up
- label={label || choice}
+ label={truncateLabel(label || choice, 30)}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@front_end/src/app/`(main)/questions/[id]/components/multiple_choices_chart_view/choices_legend/index.tsx
around lines 85 - 89, The legend popover/dropdown still renders full labels for
hidden choices; update the popover item rendering to use the same truncation as
the pinned chips by passing truncateLabel(label || choice, 30) instead of label
|| choice. Locate the popover/dropdown mapping where hidden legend items are
rendered (the code near legendChoices.map / ChoiceCheckbox in
choices_legend/index.tsx) and replace the raw label with truncateLabel(...),
ensuring any component props or keys that display text (e.g., ChoiceCheckbox or
the dropdown item renderer) receive the truncated string.
Comment on lines
253
to
+257
| const embedChoiceItems = useMemo(() => { | ||
| if (!embedMode) return choiceItems; | ||
| const othersLabel = "Others"; | ||
|
|
||
| return buildEmbedChoicesWithOthers( | ||
| choiceItems, | ||
| maxVisibleCheckboxes, | ||
| othersLabel | ||
| ); | ||
| }, [choiceItems, embedMode, maxVisibleCheckboxes]); | ||
| return buildChoicesWithOthers(choiceItems); | ||
| }, [choiceItems, embedMode]); |
Contributor
There was a problem hiding this comment.
Localize the embed "Others" label.
buildChoicesWithOthers() falls back to a hardcoded English label, and this embed path passes that item straight into MultipleChoiceTile, so the legend will show English text in every locale. Reuse the translated label here, like the tooltip path already does.
🌐 Suggested fix
const embedChoiceItems = useMemo(() => {
if (!embedMode) return choiceItems;
-
- return buildChoicesWithOthers(choiceItems);
- }, [choiceItems, embedMode]);
+ const inactiveCount = choiceItems.filter((c) => !c.active).length;
+ return buildChoicesWithOthers(
+ choiceItems,
+ t("othersCount", { count: inactiveCount })
+ );
+ }, [choiceItems, embedMode, t]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@front_end/src/components/detailed_question_card/detailed_question_card/multiple_choice_chart_card.tsx`
around lines 253 - 257, The embed path currently uses
buildChoicesWithOthers(choiceItems) which inserts a hardcoded English "Others"
label causing the legend to remain untranslated; update the embed branch in
embedChoiceItems to pass the localized label (reuse the same i18n key/translator
used by the tooltip path) into buildChoicesWithOthers or map the returned
"Others" item label through the component's translator before passing to
MultipleChoiceTile so the legend uses the translated string; reference the
embedChoiceItems constant, buildChoicesWithOthers, and MultipleChoiceTile when
making this change.
Comment on lines
+46
to
+66
| const aggTs = inactive[0]?.aggregationTimestamps ?? []; | ||
| const userTs = inactive[0]?.userTimestamps ?? []; | ||
|
|
||
| const sumNullable = (vals: Array<number | null | undefined>) => { | ||
| let sum = 0; | ||
| let hasAny = false; | ||
| for (const v of vals) { | ||
| if (v != null) { | ||
| sum += v; | ||
| hasAny = true; | ||
| } | ||
| } | ||
| return hasAny ? Number(sum.toFixed(6)) : null; | ||
| }; | ||
|
|
||
| const aggregationValues = aggTs.map((_, i) => | ||
| sumNullable(inactive.map((o) => o.aggregationValues[i])) | ||
| ); | ||
| const userValues = userTs.map((_, i) => | ||
| sumNullable(inactive.map((o) => o.userValues[i])) | ||
| ); |
Contributor
There was a problem hiding this comment.
Don't aggregate "Others" by array position.
This assumes every inactive choice shares aggregationTimestamps and userTimestamps with inactive[0], then sums ...Values[i] by index. generateChoiceItemsFromGroupQuestions() in this same file builds those timestamp arrays per question, so different close times or sparse histories will misalign the series and produce incorrect "Others" values.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@front_end/src/utils/questions/choices.ts` around lines 46 - 66, The code
incorrectly assumes all inactive entries share the same
aggregationTimestamps/userTimestamps by using inactive[0] (aggTs, userTs) and
summing aggregationValues[i]/userValues[i]; instead build a unified timestamp
set from all inactive[].aggregationTimestamps and all inactive[].userTimestamps,
create per-entry maps from timestamp->value, and then compute aggregationValues
and userValues by iterating that unified timestamp list and summing values from
each map (using null for missing entries) so timestamps are aligned across
different histories; update the logic that uses aggTs/userTs, sumNullable,
aggregationValues and userValues accordingly and keep
generateChoiceItemsFromGroupQuestions()’s timestamp semantics consistent.
Comment on lines
+201
to
+214
| const isCPHidden = !!hideCP || !!cpRevealsOn; | ||
| const needsActiveLimit = activeCount && activeCount < choiceItems.length; | ||
| const effectiveActiveCount = needsActiveLimit | ||
| ? getEffectiveVisibleCount(allOptions.length, activeCount) | ||
| : choiceItems.length; | ||
|
|
||
| // Mode 1: CP hidden → definition order, first N active | ||
| if (isCPHidden) { | ||
| if (needsActiveLimit) { | ||
| choiceItems.forEach((item, idx) => { | ||
| item.active = idx < effectiveActiveCount; | ||
| }); | ||
| } | ||
| return choiceItems; |
Contributor
There was a problem hiding this comment.
Keep the CP-based selected set when CP is hidden.
This branch now activates the first N definition-order options. The requirement only changes presentation order when CP is hidden or unrevealed; it does not change which options are selected. As written, hiding CP can swap the visible options entirely instead of just reordering the chosen set.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@front_end/src/utils/questions/choices.ts` around lines 201 - 214, The current
isCPHidden branch is mutating item.active to make the first N definition-order
items active, which swaps the selected set; instead, when isCPHidden
(cpRevealsOn or hideCP logic) do not change any item.active flags — preserve the
existing CP-based selected set — and only adjust presentation order/visible
slice if needed; so in the isCPHidden branch remove the choiceItems.forEach
mutation that sets item.active, keep computing effectiveActiveCount via
getEffectiveVisibleCount for presentation purposes, and return choiceItems (or a
reordered/trimmed view) without altering any item.active properties.
ncarazon
approved these changes
Mar 12, 2026
Comment on lines
+43
to
+56
| useEffect(() => { | ||
| if (isDropdownOpen) return; | ||
| setPinnedChoices((prev) => { | ||
| const next = new Set(prev); | ||
| let changed = false; | ||
| choices.forEach((c) => { | ||
| if (c.active && !next.has(c.choice)) { | ||
| next.add(c.choice); | ||
| changed = true; | ||
| } | ||
| }); | ||
| return changed ? next : prev; | ||
| }); | ||
| }, [choices, isDropdownOpen]); |
Contributor
There was a problem hiding this comment.
The new pinnedChoices state only adds choices but never removes them. If a user toggles many different choices on/off over time, the legend accumulates all ever-active choices. Is this intentional?
Contributor
Author
There was a problem hiding this comment.
Yes intentional.
Any item clicked on from the others list promotes to the main legend after the dropdown closes so that users can see the names and colors of the visible items. If users uncheck the legend item, it shouldn't be instantly hidden because maybe they will want to activate it again quickly after, so hiding it instantly will be a bad UX for users. -- Solution could be adding timeout and hiding after that but I found that unnecessary as it will be returned to original state after page navigation.
Comment on lines
328
to
330
| const sortedAggregationTimestamps = uniq(aggregationTimestamps).sort( | ||
| (a, b) => a - b | ||
| ); |
Contributor
There was a problem hiding this comment.
Resorts already sorted timestamps.
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
2 participants
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.
fixes #4318
Summary by CodeRabbit
New Features
Bug Fixes
Refactor