Build the title row (and bump the title size)#4647
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)
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.
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/app/(main)/questions/[id]/components/question_view/forecaster_question_view/question_header/index.tsx (1)
9-40:⚠️ Potential issue | 🟡 MinorRemove unused
QuestionHeaderInfoimport from Line 9The import is dead code.
QuestionHeaderInfois imported but never rendered or used in the component's JSX body (Lines 25-40) or in any of its dependencies. TitleRow does not reference it either. Remove the import statement.The question metadata is now displayed through
TitleRowandQuestionHeaderCPStatusinstead, so this is a cleanup task from the refactoring mentioned in the commit message ("move question info on top in forecaster header") — no functional loss.🤖 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/question_view/forecaster_question_view/question_header/index.tsx around lines 9 - 40, The import QuestionHeaderInfo is unused in the QuestionHeader component; remove the dead import statement (the line importing QuestionHeaderInfo) so only necessary imports remain—verify the QuestionHeader component (which renders PostStatusBox, MetaRow and TitleRow with variant="forecaster") still functions and that no other references to QuestionHeaderInfo exist in this file.
🧹 Nitpick comments (2)
front_end/src/app/(main)/questions/[id]/components/question_page_shell/title_row.tsx (2)
44-76: Duplicated title class string.The
"text-2xl font-bold tracking-tight sm:text-3xl lg:text-4xl"Tailwind string is repeated at Line 44 and Line 71. Consider extracting to a local constant so future typography bumps stay in sync across the forecaster and fallback branches.🤖 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/question_page_shell/title_row.tsx around lines 44 - 76, The Tailwind class string for the title is duplicated in the two JSX branches; extract it into a local constant (e.g., TITLE_CLASS) and use that constant in both places where QuestionTitle is rendered (the first branch where QuestionTitle has className="text-2xl font-bold tracking-tight sm:text-3xl lg:text-4xl" and the fallback return where cn("text-2xl font-bold tracking-tight sm:text-3xl lg:text-4xl", className) is used) so both the forecaster and fallback branches reference the same class string; update the cn(...) call to use the new TITLE_CLASS plus className.
42-54: Dead ordering classes on inner div.
lg:order-0 order-1at Line 43 sits on the only child of theflex flex-1 flex-colcontainer at Line 42, so theorder-*utilities have no effect here. The meaningful ordering is already applied via theclassNameprop on the outer wrapper (Line 37-40) by the forecaster caller. Consider dropping these classes to avoid confusion.♻️ Proposed cleanup
<div className="flex flex-1 flex-col"> - <div className="lg:order-0 order-1 flex items-center"> + <div className="flex items-center"> <QuestionTitle className="text-2xl font-bold tracking-tight sm:text-3xl lg:text-4xl">🤖 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/question_page_shell/title_row.tsx around lines 42 - 54, The inner div that wraps QuestionTitle and QuestionHeaderCPStatus contains dead ordering classes "lg:order-0 order-1" which have no effect because it's the only child of the parent "flex flex-1 flex-col"; remove those classes from that div (the element that currently has lg:order-0 order-1) so the layout relies on the outer wrapper’s className passed by the forecaster caller; verify visually that QuestionTitle, QuestionHeaderCPStatus and the isContinuousQuestion-based hideLabel behavior remain unchanged after removal.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@front_end/src/app/`(main)/questions/[id]/components/question_view/forecaster_question_view/question_header/index.tsx:
- Around line 9-40: The import QuestionHeaderInfo is unused in the
QuestionHeader component; remove the dead import statement (the line importing
QuestionHeaderInfo) so only necessary imports remain—verify the QuestionHeader
component (which renders PostStatusBox, MetaRow and TitleRow with
variant="forecaster") still functions and that no other references to
QuestionHeaderInfo exist in this file.
---
Nitpick comments:
In
`@front_end/src/app/`(main)/questions/[id]/components/question_page_shell/title_row.tsx:
- Around line 44-76: The Tailwind class string for the title is duplicated in
the two JSX branches; extract it into a local constant (e.g., TITLE_CLASS) and
use that constant in both places where QuestionTitle is rendered (the first
branch where QuestionTitle has className="text-2xl font-bold tracking-tight
sm:text-3xl lg:text-4xl" and the fallback return where cn("text-2xl font-bold
tracking-tight sm:text-3xl lg:text-4xl", className) is used) so both the
forecaster and fallback branches reference the same class string; update the
cn(...) call to use the new TITLE_CLASS plus className.
- Around line 42-54: The inner div that wraps QuestionTitle and
QuestionHeaderCPStatus contains dead ordering classes "lg:order-0 order-1" which
have no effect because it's the only child of the parent "flex flex-1 flex-col";
remove those classes from that div (the element that currently has lg:order-0
order-1) so the layout relies on the outer wrapper’s className passed by the
forecaster caller; verify visually that QuestionTitle, QuestionHeaderCPStatus
and the isContinuousQuestion-based hideLabel behavior remain unchanged after
removal.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0bf567c2-1ba2-4519-8158-49a6ce6bd662
📒 Files selected for processing (3)
front_end/src/app/(main)/questions/[id]/components/question_page_shell/title_row.tsxfront_end/src/app/(main)/questions/[id]/components/question_view/consumer_question_view/index.tsxfront_end/src/app/(main)/questions/[id]/components/question_view/forecaster_question_view/question_header/index.tsx
Cleanup: Preview Environment RemovedThe preview environment for this PR has been destroyed.
Cleanup triggered by PR close at 2026-04-23T15:26:41Z |
ncarazon
merged commit 8f7d57e
into
feat/question-page-redesign-1st-iteration
2 participants
Related to #4638
This PR extracts and builds the new Title Row layout for the updated question page shell infrastructure. Implemented features & fixes:
TitleRowcomponent handling title structures for both forecaster and consumer variantsBefore
After
Summary by CodeRabbit