-
Notifications
You must be signed in to change notification settings - Fork 27
Homepage Improvements #4123
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Homepage Improvements #4123
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThis pull request updates translations across six languages (simplifying Metaculus descriptions and adjusting terminology), refactors the homepage tab system to prioritize news content, introduces a new HomepagePostCard component, restructures the "Why Metaculus" layout, and enhances the Tabs UI component with a contained variant. Changes
Sequence DiagramsequenceDiagram
participant Page as Page<br/>(page.tsx)
participant API as ServerPostsApi
participant HF as HomePageForecasts
participant Tabs as Tabs Component
participant HPC as HomepagePostCard
participant UI as Post Rendering
Page->>API: getPostsWithCP(FILTERS.news)
API-->>Page: initialNewsPosts
Page->>HF: render with initialPosts prop
HF->>Tabs: render with defaultValue="news"
Tabs-->>HF: tab change event (news/popular/new)
HF->>HF: handleTabChange(tabId)
alt Cache hit
HF->>HF: retrieve cached posts
else Cache miss
HF->>API: fetch posts for tab
API-->>HF: newPosts
HF->>HF: cache posts by tabId
end
HF->>HPC: render HomepagePostCard per post
HPC->>UI: render post with conditional content
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@front_end/messages/es.json`:
- Around line 1850-1853: Update the translation value for the key
yearsOfPrediction to match the capitalization and idiomatic phrasing of
neighboring stat labels: change the string to "Años de predicción" (capitalized
first letter and singular "predicción") so it reads consistently with "Preguntas
abiertas" and "Pronósticos enviados" in the es.json file.
In `@front_end/src/app/`(main)/(home)/components/homepage_forecasts.tsx:
- Around line 38-56: handleTabChange can apply stale async responses to the
wrong tab: capture the requested tab id at request start and validate it against
a persisted ref that always reflects the current active tab before mutating
state. Add a useRef (e.g., activeTabRef) updated whenever setActiveTab is called
(or on effect when activeTab changes), then inside the startTransition async
block compare activeTabRef.current === id before calling setCachedPosts and
setPosts (and skip updating if it no longer matches); keep the existing
cachedPosts fast-return logic but ensure the async branch performs this check to
avoid overwriting the active tab's posts with stale responses.
🧹 Nitpick comments (1)
front_end/src/components/ui/tabs/index.tsx (1)
80-101: Contained variant may clip tabs on narrow viewports.Since contained removes horizontal overflow, tabs can become inaccessible when the list is wider than the container. Please verify on small screens; consider adding overflow or wrapping for contained mode.
♻️ Optional: keep contained tabs scrollable
- contained - ? "" // no negative margins, overflow, or background for contained variant + contained + ? "overflow-x-auto" // keep tabs accessible on narrow widths
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
front_end/messages/cs.jsonfront_end/messages/en.jsonfront_end/messages/es.jsonfront_end/messages/pt.jsonfront_end/messages/zh-TW.jsonfront_end/messages/zh.jsonfront_end/src/app/(main)/(home)/components/homepage_filters.tsfront_end/src/app/(main)/(home)/components/homepage_forecasts.tsxfront_end/src/app/(main)/(home)/components/homepage_post_card.tsxfront_end/src/app/(main)/(home)/components/why_metaculus.tsxfront_end/src/app/(main)/(home)/page.tsxfront_end/src/components/consumer_post_card/basic_consumer_post_card.tsxfront_end/src/components/ui/tabs/index.tsx
🧰 Additional context used
🧬 Code graph analysis (4)
front_end/src/app/(main)/(home)/components/homepage_post_card.tsx (3)
front_end/src/types/post.ts (1)
PostWithForecasts(214-218)front_end/src/utils/navigation.ts (1)
getPostLink(48-74)front_end/src/utils/questions/helpers.ts (3)
isQuestionPost(28-30)isMultipleChoicePost(22-26)isGroupOfQuestionsPost(32-38)
front_end/src/app/(main)/(home)/components/why_metaculus.tsx (2)
front_end/src/app/(main)/(home)/components/featured-in-logos.tsx (6)
NasdaqLogo(40-79)ForbesLogo(139-151)TheAtlanticLogo(81-137)AeiLogo(3-38)TheEconomistLogo(153-180)BloombergLogo(182-230)front_end/src/utils/formatters/number.ts (1)
abbreviatedNumber(48-98)
front_end/src/app/(main)/(home)/components/homepage_forecasts.tsx (3)
front_end/src/types/post.ts (1)
PostWithForecasts(214-218)front_end/src/app/(main)/(home)/components/homepage_filters.ts (3)
TabId(5-5)FILTERS(22-48)TABS(7-11)front_end/src/components/ui/tabs/index.tsx (3)
Tabs(37-78)TabsList(80-107)TabsTab(120-204)
front_end/src/app/(main)/(home)/page.tsx (1)
front_end/src/app/(main)/(home)/components/homepage_filters.ts (1)
FILTERS(22-48)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build Docker Image
- GitHub Check: Frontend Checks
- GitHub Check: Backend Checks
- GitHub Check: integration-tests
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (17)
front_end/messages/zh-TW.json (1)
1847-1850: LGTM — concise, consistent wording.The updated phrasing is clear and aligns with the simplified homepage copy direction.
front_end/messages/zh.json (1)
1852-1855: LGTM — concise copy update.The simplified description and label read well and match the intended messaging.
front_end/messages/cs.json (1)
1850-1853: LGTM — clean Czech localization update.The shortened description and updated label look good and stay consistent with the cross‑locale wording.
front_end/messages/en.json (1)
7-10: Copy updates look consistent with the new homepage messaging.No issues spotted; the shorter description and lowercase labels align with the refreshed tone.
front_end/messages/pt.json (1)
1848-1851: PT translation adjustments look good.Concise wording aligns with the updated English copy and keeps the same meaning.
front_end/src/components/consumer_post_card/basic_consumer_post_card.tsx (1)
74-79: No behavior change in notebook title styling.Removal of inline comments preserves the same classes; looks good.
front_end/src/app/(main)/(home)/components/why_metaculus.tsx (3)
19-58: Featured‑in logo styling updates look consistent.No issues with the revised sizing/colors.
95-152: Stats layout + abbreviated numbers look good.Compact formatting and the two‑column structure read well.
157-163: Consider allowing label wrapping for international locales.The
text-nowrapclass prevents wrapping and can cause overflow with longer translated text. Allow the label to wrap naturally by usingwhitespace-normalinstead.♻️ Suggested tweak
- <span className="text-nowrap">{label}</span> + <span className="whitespace-normal">{label}</span>front_end/src/app/(main)/(home)/components/homepage_filters.ts (1)
5-10: Tab order update aligns with news‑first navigation.Looks consistent and low risk.
front_end/src/app/(main)/(home)/page.tsx (1)
30-67: News‑first data source wiring looks consistent.Renaming to
initialNewsPostsand passinginitialPostslines up with the new tab order.front_end/src/app/(main)/(home)/components/homepage_post_card.tsx (1)
62-65: Add an accessible label to the full‑card Link overlay.The overlay link is focusable but has no name; screen readers will announce an unlabeled link. Add an
aria-label(e.g., the post title).🐛 Proposed fix
- <Link - href={getPostLink(post)} - className="absolute top-0 z-100 h-full w-full `@container`" - /> + <Link + href={getPostLink(post)} + aria-label={title} + className="absolute top-0 z-100 h-full w-full `@container`" + />front_end/src/app/(main)/(home)/components/homepage_forecasts.tsx (5)
7-15: LGTM on Tabs + HomepagePostCard integration.Imports are consistent with the new tab system and card component usage.
16-31: State and cache initialization look solid.Initializing
newsas the default and preloading the cache withinitialPostsmatches the new data flow.
64-87: Tabs UI wiring looks good.The contained TabsList and styling tweaks align well with the new layout.
89-100: Grid-to-columns rendering is clean and consistent.HomepagePostCard usage and the responsive column layout look well aligned with the new design.
108-118: ExploreAllCard styling update looks good.Spacing and borders align with the new card layout.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| "metaculusDescription": "Metaculus es una plataforma de pronósticos en línea enfocada en temas de importancia global.", | ||
| "openQuestions": "Preguntas abiertas", | ||
| "forecastsSubmitted": "Pronósticos enviados", | ||
| "yearsOfPrediction": "Años de predicción", | ||
| "yearsOfPrediction": "años de predicciones", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
head -n 1860 front_end/messages/es.json | tail -n 30 | cat -nRepository: Metaculus/metaculus
Length of output: 2333
🏁 Script executed:
sed -n '1840,1860p' front_end/messages/es.json | cat -nRepository: Metaculus/metaculus
Length of output: 1592
🏁 Script executed:
rg -n "forecastsSubmitted|openQuestions|yearsOfPrediction|Preguntas|Pronósticos" front_end/messages/es.json | head -20Repository: Metaculus/metaculus
Length of output: 1709
Correct yearsOfPrediction casing and phrasing for consistency with adjacent stat labels.
Neighboring stat labels ("Preguntas abiertas", "Pronósticos enviados") start with capitalized nouns, but "años de predicciones" begins with lowercase. Additionally, the singular form "Años de predicción" is more idiomatic in Spanish for UI metric labels than the plural.
Suggested adjustment
- "yearsOfPrediction": "años de predicciones",
+ "yearsOfPrediction": "Años de predicción",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "metaculusDescription": "Metaculus es una plataforma de pronósticos en línea enfocada en temas de importancia global.", | |
| "openQuestions": "Preguntas abiertas", | |
| "forecastsSubmitted": "Pronósticos enviados", | |
| "yearsOfPrediction": "Años de predicción", | |
| "yearsOfPrediction": "años de predicciones", | |
| "metaculusDescription": "Metaculus es una plataforma de pronósticos en línea enfocada en temas de importancia global.", | |
| "openQuestions": "Preguntas abiertas", | |
| "forecastsSubmitted": "Pronósticos enviados", | |
| "yearsOfPrediction": "Años de predicción", |
🤖 Prompt for AI Agents
In `@front_end/messages/es.json` around lines 1850 - 1853, Update the translation
value for the key yearsOfPrediction to match the capitalization and idiomatic
phrasing of neighboring stat labels: change the string to "Años de predicción"
(capitalized first letter and singular "predicción") so it reads consistently
with "Preguntas abiertas" and "Pronósticos enviados" in the es.json file.
| const handleTabChange = (tabId: string) => { | ||
| const id = tabId as TabId; | ||
| if (id === activeTab) return; | ||
|
|
||
| setActiveTab(tabId); | ||
| setActiveTab(id); | ||
|
|
||
| if (cachedPosts[tabId]) { | ||
| setPosts(cachedPosts[tabId] ?? []); | ||
| if (cachedPosts[id]) { | ||
| setPosts(cachedPosts[id] ?? []); | ||
| return; | ||
| } | ||
|
|
||
| startTransition(async () => { | ||
| const response = await ClientPostsApi.getPostsWithCPForHomepage( | ||
| FILTERS[tabId] | ||
| FILTERS[id] | ||
| ); | ||
| const newPosts = response.results; | ||
| setCachedPosts((prev) => ({ ...prev, [tabId]: newPosts })); | ||
| setCachedPosts((prev) => ({ ...prev, [id]: newPosts })); | ||
| setPosts(newPosts); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
fd "homepage_forecasts.tsx" --type fRepository: Metaculus/metaculus
Length of output: 128
🏁 Script executed:
cat -n front_end/src/app/(main)/(home)/components/homepage_forecasts.tsxRepository: Metaculus/metaculus
Length of output: 222
🏁 Script executed:
cat -n "front_end/src/app/(main)/(home)/components/homepage_forecasts.tsx"Repository: Metaculus/metaculus
Length of output: 4658
Guard against stale async responses overriding the active tab.
If a user switches tabs quickly, a slower request can finish later and overwrite posts for the wrong tab. When the response for "popular" tab arrives after the user has switched to "new" tab, line 55 will still call setPosts(newPosts) with the stale data, causing the displayed content to jump back to the previous tab.
Add a ref to track the active tab at response completion time and validate the response still matches the current tab before updating state:
Proposed fix
-import { FC, useState, useTransition } from "react";
+import { FC, useEffect, useRef, useState, useTransition } from "react";
const [activeTab, setActiveTab] = useState<TabId>("news");
+ const activeTabRef = useRef<TabId>("news");
+
+ useEffect(() => {
+ activeTabRef.current = activeTab;
+ }, [activeTab]);
const handleTabChange = (tabId: string) => {
const id = tabId as TabId;
if (id === activeTab) return;
@@
startTransition(async () => {
const response = await ClientPostsApi.getPostsWithCPForHomepage(
FILTERS[id]
);
const newPosts = response.results;
setCachedPosts((prev) => ({ ...prev, [id]: newPosts }));
- setPosts(newPosts);
+ if (activeTabRef.current !== id) return;
+ setPosts(newPosts);
});
};🤖 Prompt for AI Agents
In `@front_end/src/app/`(main)/(home)/components/homepage_forecasts.tsx around
lines 38 - 56, handleTabChange can apply stale async responses to the wrong tab:
capture the requested tab id at request start and validate it against a
persisted ref that always reflects the current active tab before mutating state.
Add a useRef (e.g., activeTabRef) updated whenever setActiveTab is called (or on
effect when activeTab changes), then inside the startTransition async block
compare activeTabRef.current === id before calling setCachedPosts and setPosts
(and skip updating if it no longer matches); keep the existing cachedPosts
fast-return logic but ensure the async branch performs this check to avoid
overwriting the active tab's posts with stale responses.
🚀 Preview EnvironmentYour preview environment is ready!
Details
ℹ️ Preview Environment InfoIsolation:
Limitations:
Cleanup:
|
Summary by CodeRabbit
New Features
Style
✏️ Tip: You can customize this high-level summary in your review settings.