Fix question page redesign QA fixes#4774
Draft
ncarazon wants to merge 2 commits into
Draft
Conversation
…ng, chart height sync, key factor count and placeholder styling, resolution criteria drawer, chip truncation/click/resolved label, and hover stuck-state
Contributor
|
Important Review skippedDraft detected. 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)
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/app/(main)/questions/[id]/components/question_view/consumer_question_view/consumer_list_chart_shell.tsx (1)
52-53: ⚡ Quick winCentralize the chart-height offset contract.
40is part of a cross-component sizing contract; keeping it as a local literal makes drift likely as chart header/toggle heights evolve. Consider moving these offsets into shared constants (or measured refs) used by shell + chart panels.🤖 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 52 - 53, The hardcoded offset "40" used when computing chartAreaHeight in consumer_list_chart_shell.tsx is part of a cross-component sizing contract and should be centralized; replace the literal with a shared constant (e.g., CHART_HEADER_TOGGLE_OFFSET) exported from a shared layout/constants module (or derive it from a measured ref exposed by the chart panel) and use that constant in consumer_list_chart_shell.tsx and the corresponding chart panels so both sides consume the same value (update references to chartAreaHeight and any chart panels that assume the same offset).
🤖 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/app/`(main)/questions/[id]/components/key_factors/key_factors_grid_placeholder.tsx:
- Around line 19-28: The placeholder uses a clickable div with onClick in
key_factors_grid_placeholder.tsx which breaks keyboard accessibility; change the
interactive variant to a semantic button (e.g., render a <button type="button">
when onClick is present) so it is focusable and keyboard-activatable, preserve
all className, onClick prop, and any aria attributes, and remove the
non-semantic onClick-div pattern; if for some reason you must keep a div, add
role="button", tabIndex={0} and handle keyDown (Enter/Space) to call the same
onClick handler, but prefer replacing the div with a button for correct
semantics.
In
`@front_end/src/app/`(main)/questions/[id]/components/question_page_shell/meta_row.tsx:
- Around line 40-51: The allProjects construction currently only filters out
defaultProject?.id, so the same project appearing in multiple projectsData
buckets can be duplicated; update the logic that builds allProjects (using
projectsData, defaultProject) to perform full deduplication by project id across
all buckets—e.g., iterate through the concatenated arrays (defaultProject first
if present, then the various projectsData arrays) and include each project only
if its id has not been seen before (use a Set of ids or a Map keyed by id) so
that items appearing in multiple buckets are de-duplicated and chips/"N more"
counts are accurate.
In
`@front_end/src/components/consumer_post_card/group_forecast_card/forecast_card_wrapper.tsx`:
- Around line 66-70: The footer fade uses a hardcoded light color
(METAC_COLORS.gray[0].DEFAULT) which breaks dark mode; update the element inside
ForecastCardWrapper (the div with className "pointer-events-none absolute
bottom-0 left-0 right-0 z-10 h-8") to use a theme-aware color instead of the
fixed value: detect the current theme (e.g., via the existing theme/context hook
or a useIsDarkMode utility) or reference a CSS variable for background, and set
the linear-gradient stop to the appropriate light or dark token (or tailwind
theme class) so the fade matches both light and dark modes. Ensure you replace
the inline METAC_COLORS.gray[0].DEFAULT usage with that theme-aware value.
In `@front_end/src/hooks/use_overlay_max_height.ts`:
- Around line 14-24: The current useEffect only sets overlayMaxHeight once on
expand, so the cap grows stale when the viewport changes; modify the effect
around expanded/containerRef to recompute and setOverlayMaxHeight whenever the
window resizes or the page scrolls while expanded by adding window 'resize' (and
optionally 'scroll') event listeners that call the same height calculation
(using containerRef.current.getBoundingClientRect(), OVERLAY_OFFSET,
BOTTOM_MARGIN, MIN_HEIGHT) and cleaning them up on unmount or when expanded
becomes false; ensure the handler is the same function reference for removal and
guard containerRef.current existence before calling setOverlayMaxHeight.
---
Nitpick comments:
In
`@front_end/src/app/`(main)/questions/[id]/components/question_view/consumer_question_view/consumer_list_chart_shell.tsx:
- Around line 52-53: The hardcoded offset "40" used when computing
chartAreaHeight in consumer_list_chart_shell.tsx is part of a cross-component
sizing contract and should be centralized; replace the literal with a shared
constant (e.g., CHART_HEADER_TOGGLE_OFFSET) exported from a shared
layout/constants module (or derive it from a measured ref exposed by the chart
panel) and use that constant in consumer_list_chart_shell.tsx and the
corresponding chart panels so both sides consume the same value (update
references to chartAreaHeight and any chart panels that assume the same offset).
🪄 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: afcf0256-f0ff-47ca-ad46-d6eeb4a7c1a8
📒 Files selected for processing (17)
front_end/src/app/(main)/questions/[id]/[[...slug]]/page_component.tsxfront_end/src/app/(main)/questions/[id]/components/key_factors/key_factors_feed.tsxfront_end/src/app/(main)/questions/[id]/components/key_factors/key_factors_grid_placeholder.tsxfront_end/src/app/(main)/questions/[id]/components/multiple_choices_chart_view/compact_legend_bar/index.tsxfront_end/src/app/(main)/questions/[id]/components/post_score_data/single_question_score_data.tsxfront_end/src/app/(main)/questions/[id]/components/question_page_shell/index.tsxfront_end/src/app/(main)/questions/[id]/components/question_page_shell/meta_row.tsxfront_end/src/app/(main)/questions/[id]/components/question_page_shell/tab_bar.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/fan_graph_chart_panel.tsxfront_end/src/app/(main)/questions/[id]/components/question_view/consumer_question_view/timeline/index.tsxfront_end/src/components/consumer_post_card/group_forecast_card/forecast_card_wrapper.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/hooks/use_overlay_max_height.tsfront_end/src/hooks/use_post_text_sections.ts
💤 Files with no reviewable changes (1)
- front_end/src/app/(main)/questions/[id]/components/post_score_data/single_question_score_data.tsx
…eight on expand, and recompute overlay max height on resize
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
This PR addresses a batch of QA-reported issues on the question page redesign across forecaster and consumer views.
Implemented:
meta_row.tsx): always showsdefault_projectas the single visible chip; remaining projects collapsed under "{n} more…"question_page_shell,timeline/index.tsx): forecaster and consumer shells now renderConditionalTimelinefor conditional postsuse_overlay_max_height.ts, forecast cards): new hook caps expanded overlay to viewport bottom; content scrollable with fade gradient, collapse button always pinned visibleconsumer_list_chart_shell, chart components): list column height propagated via context aschartAreaHeight; charts dynamically fill available space, scaling down between 1024–1280 pxtab_bar.tsx): tab badge now includesaggregateCoherenceLinkscount alongsidekey_factorsquestion_page_shell,use_post_text_sections): fine print stays inside the resolution criteria drawer in forecaster mode, matching prod behaviourkey_factors_grid_placeholder): 50% opacity by default, full opacity + border on hover; always shows "+ Add Key Factor"; triggers login modal for unauthenticated userscompact_legend_bar): max-width widened to 300 px; full chip click toggles visibility; resolved winner shows(Yes)instead of forecast %;showAllorders winner firstcompact_legend_bar): removeditem.highlightedfrom chip className, CSShover:handles visual state, eliminating stuck-highlight race conditionDemo videos
tag-truncation.mp4
extendable-section.mp4
left-pane-scale-down.mp4
fill-the-available-height.mp4
key-factor-correct-count.mp4
updated-placeholder.mp4
fine-print.mp4
mc-checkbox.mp4
winning-option.mp4
Summary by CodeRabbit
New Features
Bug Fixes & UI/UX Improvements