Conversation
…ncies - Introduced a new Labor Automation Hub page with sections for activity monitoring, job statistics, and methodology. - Added new components including ActivityCard, InfoPopover, LaborHubHeader, and TabGroup for enhanced UI functionality. - Integrated new data structures for risk and job forecasts. - Included new images for partner logos and added the `react-scrollspy-navigation` package for scroll navigation. - Updated package.json and package-lock.json to reflect new dependencies.
- Removed `react-scrollspy-navigation` from package.json and package-lock.json as it was not utilized in the project. - Updated the Tailwind configuration to support a more complex dark mode strategy. - Refactored the Labor Automation Hub page to incorporate new UI components such as DualPaneSectionCard, TableCompact, and HeroSection for improved layout and functionality. - Enhanced the LaborHubHeader and TabGroup components for better navigation experience. - Introduced InvertedThemeContainer for managing theme context in the application.
- Deleted unused partner images: `renphil-dark.png`, `renphil-light.webp`, and `sff-light.png`. - Updated `layout.tsx` to include `TailwindIndicator` for development visibility. - Refactored `LaborAutomationHubPage` to integrate `LaborHubNavigation` for improved navigation and layout consistency. - Removed `LaborHubHeader` and `LaborHubInfo` components, streamlining the page structure. - Enhanced `HeroSection` and `TabGroup` components for better responsiveness and visual appeal.
- Added a new Tailwind configuration file `tailwind.config.prettier.ts` to manage dark mode variants without altering class order. - Updated `.prettierrc.json` to include the new Tailwind configuration file. - Refactored `LaborHubNavigation` and `HeroSection` components for improved styling consistency and responsiveness.
- Introduced new OverviewSection and ResearchSection components for better content organization. - Updated layout and styling for various components, including ActivityMonitorSection and DualPaneSectionCard, to enhance responsiveness and visual appeal. - Added ContentParagraph component for consistent text formatting across sections. - Adjusted tab labels and removed outdated sections to reflect current content focus.
…atures - Introduced BasicQuestion and BarChart components for improved data visualization and interaction. - Updated LaborAutomationHubPage to utilize new components, enhancing the display of featured posts and questions. - Refactored ResearchSection to include AI vulnerability ratings and updated employment change data. - Improved the HeroSection with better SVG attributes for consistency. - Added QuestionCardContainer for better layout management of question cards.
- Removed the BarChart and QuestionCardContainer components to simplify the structure. - Introduced a new QuestionCard component for better layout and styling of question-related content. - Integrated RiskChart into the OverviewSection and ActivityMonitorSection for improved data representation. - Updated ActivityMonitorSection to utilize the new QuestionCard for displaying projected employment growth.
…tion - Introduced MultiLineRiskChart for enhanced data visualization of employment changes. - Updated OverviewSection to include the new chart, replacing the placeholder div for improved content representation. - Refactored LaborHubNavigation for better styling consistency.
…ance question card components - Replaced BasicQuestion component with QuestionLoader for improved data fetching and rendering of questions. - Introduced FlippableQuestionCard for enhanced user interaction with question content. - Added QuestionCardSkeleton for loading states while fetching question data. - Updated ActivityMonitorSection to use QuestionLoader for displaying projected employment growth. - Streamlined question card structure for better maintainability and performance.
…ctionality - Added a MoreButton to the QuestionCard component, providing users with options to view, export, share, and copy links. - Updated the QuestionCard to conditionally render the MoreButton based on the new showMoreButton prop. - Introduced POST_IDS_FILTER in filters to support filtering by IDs in search parameters. - Enhanced filters logic to handle both array and comma-separated string inputs for IDs.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR introduces a comprehensive "Labor Automation Hub" feature with a new Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/Browser
participant Page as Labor Hub Page
participant Sections as Section Components
participant API as Data APIs
participant Chart as Chart Components
participant Hover as Chart Hover Context
User->>Page: Visit /labor-hub
Page->>API: Fetch overall employment data
Page->>API: Fetch jobs data with forecasts
API-->>Page: Return forecasted data
Page->>Sections: Render with data
Sections->>Chart: Initialize charts with data
Chart->>Hover: Provide hover context
User->>Chart: Hover over chart point
Chart->>Hover: Update hoverYear state
Hover-->>Chart: Broadcast hover state
Chart->>Chart: Highlight series/envelope
User->>Sections: Hover over activity marker
Sections->>Hover: Set hoveredActivityId
Hover-->>Chart: Activity highlight syncs
Chart->>Chart: Display marker styling
User->>Sections: Switch mobile tab
Sections->>Hover: Clear hoverYear/highlightedEnvelope
Chart->>Chart: Reset highlight state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
🚀 Preview EnvironmentYour preview environment is ready!
Details
ℹ️ Preview Environment InfoIsolation:
Limitations:
Cleanup:
|
- Refactored `MoreButton` and `QuestionCard` components to support multiple post IDs for actions like viewing, exporting, and sharing. - Enhanced the `QuestionCard` footer to include Metaculus attribution with the current date.
There was a problem hiding this comment.
Actionable comments posted: 9
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
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/download_question_data_modal/index.tsx (1)
66-93:⚠️ Potential issue | 🟠 MajorReset
dataAccessStatuswhen the selected post set changes.
isLoadedstays sticky after the first selection. If the modal is reused for a different post, or after a multi-post open, this effect can skip the new access-status request and keep the old deanonymization/whitelist state.🛠️ One way to key the status to the current selection
const [dataAccessStatus, setDataAccessStatus] = useState({ has_data_access: false, view_deanonymized_data: false, isLoaded: false, }); + useEffect(() => { + setDataAccessStatus({ + has_data_access: false, + view_deanonymized_data: false, + isLoaded: false, + }); + }, [primaryPostId, isMultiplePosts]); + useEffect(() => { // Skip whitelist status for multiple posts - not supported if (isMultiplePosts) { setDataAccessStatus({ has_data_access: false, @@ - if (!isOpen || dataAccessStatus.isLoaded) { + if (!isOpen || dataAccessStatus.isLoaded || primaryPostId === undefined) { return; }🤖 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/download_question_data_modal/index.tsx around lines 66 - 93, The effect currently skips fetching when dataAccessStatus.isLoaded is true so the flag remains sticky across different selections; ensure the status is reset when the selected post set changes (e.g., when primaryPostId or isMultiplePosts changes) by calling setDataAccessStatus to clear or set isLoaded to false (and reset has_data_access/view_deanonymized_data) before the fetch logic in the useEffect (the one using ClientPostsApi.getDataAccessStatus and setDataAccessStatus) so each new selection triggers a fresh request.
🟡 Minor comments (14)
front_end/src/app/(main)/labor-hub/sections/activity-monitor-interactive.tsx-51-53 (1)
51-53:⚠️ Potential issue | 🟡 MinorButton has no
onClickhandler and hardcoded English text.The "See all activity" button lacks an
onClickhandler, making it non-functional. If this is a placeholder for future functionality, consider adding a TODO comment or disabling the button. Additionally, the button text should use i18n for localization.🛠️ Suggested improvements
- <button className="w-full rounded-md border border-blue-400 bg-blue-100 py-3 text-center text-lg font-medium leading-7 text-blue-800 hover:bg-blue-200 dark:border-blue-400-dark dark:bg-blue-100-dark dark:text-blue-800-dark dark:hover:bg-blue-200-dark print:hidden"> - See all activity + {/* TODO: Implement activity expansion functionality */} + <button + className="w-full rounded-md border border-blue-400 bg-blue-100 py-3 text-center text-lg font-medium leading-7 text-blue-800 hover:bg-blue-200 dark:border-blue-400-dark dark:bg-blue-100-dark dark:text-blue-800-dark dark:hover:bg-blue-200-dark print:hidden" + onClick={() => { /* TODO: implement */ }} + > + {t("seeAllActivity")} </button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/sections/activity-monitor-interactive.tsx around lines 51 - 53, The button in activity-monitor-interactive.tsx currently has no onClick and uses a hardcoded English label; update the <button> in the ActivityMonitorInteractive component to either wire an onClick handler (e.g., onClick={() => navigateToActivityList()} or a passed prop callback like onSeeAllActivity) or mark it disabled and add a TODO comment if it's intentionally non-functional, and replace the hardcoded "See all activity" text with an i18n call (e.g., useTranslation/t('seeAllActivity') or <Trans> with key "seeAllActivity") so the label is localizable; ensure you add/update any props or helper functions (navigateToActivityList, onSeeAllActivity) and corresponding tests/usage where this component is rendered.front_end/src/app/(main)/labor-hub/components/print-attribution.tsx-4-7 (1)
4-7:⚠️ Potential issue | 🟡 MinorPotential hydration mismatch with
windowaccess during render.Accessing
window.locationdirectly during render causes a hydration mismatch: the server renders an empty string while the client renders the actual URL. Consider usinguseEffectwith state to set the URL only on the client after mount.🛠️ Proposed fix using useEffect
"use client"; +import { useEffect, useState } from "react"; + export function PrintAttribution() { - const url = - typeof window !== "undefined" - ? window.location.hostname + window.location.pathname - : ""; + const [url, setUrl] = useState(""); + + useEffect(() => { + setUrl(window.location.hostname + window.location.pathname); + }, []); return (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/print-attribution.tsx around lines 4 - 7, Replace the direct window access in the render of the PrintAttribution component by initializing a local state (e.g., url state) to an empty string and populating it inside a useEffect on mount; specifically, remove the top-level const url that reads window.location, add const [url, setUrl] = useState("") and in useEffect(() => { if (typeof window !== "undefined") setUrl(window.location.hostname + window.location.pathname); }, []) so the server renders"" and the client updates after hydration, avoiding mismatches.front_end/src/app/(main)/questions/helpers/filters.ts-126-137 (1)
126-137:⚠️ Potential issue | 🟡 MinorEdge case: empty string produces
[0]instead of[].When
idsParamis an empty string"","".split(",")yields[""], andNumber("")evaluates to0(notNaN), sofilters.idsbecomes[0]. This could cause unintended API requests for post ID 0.Proposed fix
} else if (typeof idsParam === "string") { // Handle comma-separated values or single value filters.ids = idsParam .split(",") + .filter((s) => s.trim() !== "") .map(Number) .filter((id) => !isNaN(id)); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/questions/helpers/filters.ts around lines 126 - 137, The current ids parsing for POST_IDS_FILTER can turn an empty string into 0 (producing filters.ids = [0]); update the logic that builds filters.ids from idsParam (both in the Array.isArray and string branches) to ignore empty or whitespace-only entries before converting to Number—e.g., trim and filter out entries where entry === "" (or length === 0) prior to mapping to Number and filtering !isNaN(id); ensure filters.ids ends up as an empty array when idsParam is "" or contains only empty segments.front_end/src/app/(main)/labor-hub/components/labor-hub-navigation.tsx-141-149 (1)
141-149:⚠️ Potential issue | 🟡 MinorLocalize the bell button's accessible name.
aria-label="Subscribe for updates"is still hardcoded English, so screen-reader users won't get a translated label here.Based on learnings: Do not hardcode English strings in TSX components. Prefer using
useTranslations()and i18n strings for UI text across components.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/labor-hub-navigation.tsx around lines 141 - 149, The bell button's accessible name is hardcoded ("Subscribe for updates"); replace the literal aria-label with a localized string by importing and calling useTranslations() (or your i18n hook) in the component and using the translation key for the subscribe label; update the Button JSX (the element using ref={setReference}, aria-expanded={isNewsletterOpen}, presentationType="icon") to set aria-label={t('...')} (or equivalent) and add the new i18n key to your locale files so screen readers get the translated label.front_end/src/app/(main)/labor-hub/components/question-cards/multi-question-line-chart-client.tsx-34-67 (1)
34-67:⚠️ Potential issue | 🟡 MinorBase the
+sign on the rounded value, not the raw value.In
percentageChangemode, values that round to zero can currently display as+0%. The sign should be computed from the formatted number.🧮 Suggested adjustment
const formatYValue = useCallback( (value: number) => { switch (valueFormat) { @@ case "percentageChange": default: { - const rounded = trimTrailingZeros(value); - const sign = value > 0 ? "+" : ""; - return `${sign}${rounded}%`; + const roundedNumber = Number(value.toFixed(decimals)); + const sign = roundedNumber > 0 ? "+" : ""; + return `${sign}${roundedNumber.toString()}%`; } } }, - [trimTrailingZeros, valueFormat] + [decimals, trimTrailingZeros, valueFormat] ); @@ case "percentageChange": default: { - if (value === 0) return "0%"; - const sign = value > 0 ? "+" : ""; - return `${sign}${value.toFixed(0)}%`; + const roundedNumber = Number(value.toFixed(0)); + if (roundedNumber === 0) return "0%"; + const sign = roundedNumber > 0 ? "+" : ""; + return `${sign}${roundedNumber}%`; } } },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question-cards/multi-question-line-chart-client.tsx around lines 34 - 67, In both formatYValue and formatYTick, compute the sign from the rounded/formatted value instead of the raw value: for formatYValue call trimTrailingZeros(value) and parse the result to a number (e.g., parseFloat) to decide the sign and then return `${sign}${rounded}%`; for formatYTick get the rounded string via value.toFixed(0), parse it to an integer/number to determine if it's zero or positive/negative (use === 0 to return "0%"), and derive the sign from that parsed rounded value before composing the final `${sign}${roundedStr}%` string.front_end/src/app/(main)/labor-hub/sections/activity-monitor.tsx-66-70 (1)
66-70:⚠️ Potential issue | 🟡 MinorPlaceholder link
href="#"should be replaced.The Reuters link uses
href="#"which is non-functional. This should either be replaced with the actual URL or removed if unavailable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/sections/activity-monitor.tsx around lines 66 - 70, In the ActivityMonitor component (front_end/src/app/(main)/labor-hub/sections/activity-monitor.tsx) replace the placeholder anchor href="#" for the Reuters link with the actual Reuters article URL (or remove the anchor if no URL is available); update the <a> element referenced in the JSX (the Reuters link) so it points to the real external URL and include target="_blank" rel="noopener noreferrer" when linking externally.front_end/src/app/(main)/labor-hub/page.tsx-424-427 (1)
424-427:⚠️ Potential issue | 🟡 MinorPlaceholder content should be replaced.
"John Doe" and "Placeholder quote from Pro about Washington specific employment changes" appear to be demo placeholders that need real content before release.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/page.tsx around lines 424 - 427, Replace the hard-coded placeholder content in the ActivityCard instance: remove "John Doe", the static subtitle "Pro Forecaster", and the placeholder child text and instead supply real data (e.g., use the record's authorName, authorTitle/subtitle and quote/content) or wire the card to the appropriate data source/props/state; specifically update the ActivityCard component usage so username, subtitle, and children come from your real data variables (or a localized copy) rather than literal demo strings.utils/views.py-145-150 (1)
145-150:⚠️ Potential issue | 🟡 MinorPotential
ValueErroron invalidpost_idformat.Line 149 calls
int(pid)without error handling. If a client passes a non-numeric string inpost_ids, this will raise an unhandledValueErrorinstead of returning a proper API error response.🛡️ Proposed fix to handle invalid post IDs gracefully
if post_ids: posts = list(Post.objects.filter(id__in=post_ids)) if len(posts) != len(post_ids): found_ids = {p.id for p in posts} - missing_ids = [pid for pid in post_ids if int(pid) not in found_ids] + try: + missing_ids = [pid for pid in post_ids if int(pid) not in found_ids] + except ValueError: + raise ValidationError("Invalid post_id format: post_ids must be integers") raise NotFound(f"Posts not found: {missing_ids}")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@utils/views.py` around lines 145 - 150, The check that computes missing_ids in the Post lookup can raise ValueError because it calls int(pid) on entries from post_ids; update the logic in the block that uses Post.objects.filter (variables: post_ids, posts, found_ids, missing_ids) to validate/convert each post_id safely (e.g., attempt to parse to int with try/except or pre-filter non-integer ids) and handle invalid formats by returning a proper API error (BadRequest or NotFound) instead of allowing an unhandled ValueError; ensure the final missing_ids comparison uses the validated integer IDs and that any parse errors produce a clear API error response.front_end/src/app/(main)/labor-hub/sections/overview.tsx-243-245 (1)
243-245:⚠️ Potential issue | 🟡 MinorUse the same precision for the displayed value and the baseline comparison.
Right now a value like
2.6renders asgrow 3%but still falls into the< 3branch, so the paragraph can say the forecast “falls short of” a+3%baseline while showing3%growth. Please compare against the same rounded value you display, or keep a decimal in the copy.♻️ Minimal fix
- const formatOverallChange = (value: number) => - `${value < 0 ? "fall" : "grow"} ${Math.abs(value).toFixed(0)}%`; + const roundedOverallChange = (value: number) => + Number(Math.abs(value).toFixed(0)); + + const formatOverallChange = (value: number) => + `${value < 0 ? "fall" : "grow"} ${roundedOverallChange(value)}%`; ... - {change2035 != null && change2035 < 0 + {change2035 != null && change2035 < 0 ? "This sharply contrasts with" - : change2035 != null && change2035 < 3 + : change2035 != null && roundedOverallChange(change2035) < 3 ? "This falls short of" : "This is in line with"}{" "}Also applies to: 268-288
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/sections/overview.tsx around lines 243 - 245, The displayed percent and the baseline comparison are inconsistent in formatOverallChange: round the numeric value once (e.g., const rounded = Math.round(value) or use a fixed decimal like const displayed = Number(value.toFixed(1))) and use that same rounded/displayed variable for both the comparison and the returned string; apply the same change to the similar formatting logic referenced in the 268-288 block so the branch condition and visible percent use identical precision.front_end/src/app/(main)/labor-hub/components/labor-hub-by-job-vulnerability-card.tsx-85-111 (1)
85-111:⚠️ Potential issue | 🟡 MinorGuard the summary sentence when 2035 data is missing.
If
mostVulnerable2035isnull, this renders asare expected to ., and the trailing clause can also disappear mid-sentence. Please only render this paragraph when the required values exist, or provide explicit fallback copy.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/labor-hub-by-job-vulnerability-card.tsx around lines 85 - 111, The paragraph currently renders incomplete text when mostVulnerable2035 (and sometimes leastVulnerable2035) is null; update the component so the entire paragraph is only rendered when required data exists: check mostVulnerable2035 != null (and if you rely on the least-vulnerable clause also require leastVulnerable2035 != null) before outputting the whole <div>, or provide explicit fallback copy instead of empty placeholders; use the existing symbols mostVulnerable2035, leastVulnerable2035, occupationForecastClassName, and formatOccupationChange to locate the logic and ensure the sentence never renders as “are expected to .” by gating or substituting a clear fallback.front_end/src/app/(main)/labor-hub/sections/sortable-research-table.tsx-113-116 (1)
113-116:⚠️ Potential issue | 🟡 MinorDon’t sort missing values as real zeroes.
row.values[key.index] ?? 0makes—cells sort exactly like an actual0%change, which is misleading once users start ordering the table. Missing values should sort separately, typically at the end.♻️ Minimal fix
-function getSortValue(row: ResearchTableRow, key: SortKey): number { - if (key.type === "rating") return row.rating; - return row.values[key.index] ?? 0; +function getSortValue( + row: ResearchTableRow, + key: SortKey +): number | null { + if (key.type === "rating") return row.rating; + return row.values[key.index] ?? null; } ... const sortedRows = useMemo(() => { return [...rows].sort((a, b) => { const aVal = getSortValue(a, sortKey); const bVal = getSortValue(b, sortKey); + if (aVal == null && bVal == null) return 0; + if (aVal == null) return 1; + if (bVal == null) return -1; return sortDirection === "desc" ? bVal - aVal : aVal - bVal; }); }, [rows, sortKey, sortDirection]);Also applies to: 155-160
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/sections/sortable-research-table.tsx around lines 113 - 116, getSortValue currently coalesces missing cells to 0 which makes empty "—" values sort like real zeroes; change getSortValue to return a distinct sentinel for missing (e.g., null or NaN) instead of 0 (handle both rating branch and the branch using row.values[key.index]), and update the table sort comparator to explicitly check for that sentinel and always treat missing values as greater than any real numeric value so they appear at the end when sorting; apply the same change to the other sort helper/usage referenced around lines 155-160.front_end/src/components/charts/primitives/timeline_markers/group_timeline_markers_overlay.tsx-63-72 (1)
63-72:⚠️ Potential issue | 🟡 MinorTranslate the fallback marker copy.
Both the tooltip fallback (
"Activity") and the aria-label fallback ("Timeline marker") are hardcoded English UI strings. Please move them into the existing i18n flow instead of baking them into the component.Based on learnings, "Do not hardcode English strings in TSX components... Prefer using useTranslations() and i18n strings for UI text across components."
Also applies to: 111-113
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/components/charts/primitives/timeline_markers/group_timeline_markers_overlay.tsx` around lines 63 - 72, Replace hardcoded fallback UI strings in GroupTimelineMarkersOverlay with i18n keys: import and use useTranslations (e.g., const t = useTranslations('namespace')) and replace the literal "Activity" used in tooltipContent fallback and the literal "Timeline marker" used for the aria-label (and the similar strings at the block around lines 111-113) with t('activityFallback') and t('timelineMarkerAria') (or equivalent keys in your i18n file); ensure you add those keys to the appropriate locale namespace so the component calls t(...) instead of using English literals.front_end/src/app/(main)/labor-hub/components/question-cards/multi-question-table.tsx-146-149 (1)
146-149:⚠️ Potential issue | 🟡 MinorLocalize the fallback static-column label.
"Current"is rendered directly whenstaticColumnHeaderis omitted, so this table will ship an English-only header on otherwise localized pages. Please route the default through i18n or require callers to pass the label explicitly.Based on learnings, "Do not hardcode English strings in TSX components... Prefer using useTranslations() and i18n strings for UI text across components."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question-cards/multi-question-table.tsx around lines 146 - 149, The component currently hardcodes the fallback label "Current" for staticColumnHeader; update the rendering to use the i18n hook (useTranslations) or make staticColumnHeader required so no hardcoded English appears. Specifically, import and call useTranslations inside the component and replace the fallback "Current" in the TableCompactHeaderCell with a translated key (e.g., t('laborHub.current') or similar), or change the prop/staticColumnHeader handling to force callers to supply the localized label instead. Ensure you reference the existing symbols staticColumnHeader, hasStaticColumn, and TableCompactHeaderCell when making the change.front_end/src/app/(main)/labor-hub/sections/jobs-monitor.tsx-107-120 (1)
107-120:⚠️ Potential issue | 🟡 MinorAdd the missing positive-end scale label.
The desktop legend renders
40% lower,20% lower,2025 Baseline,20% higher, and then leaves the last column empty. That underlabels the maximum positive bar width and makes the scale read asymmetrically.Suggested fix
- <div className="text-right text-olive-700 dark:text-olive-700-dark"></div> + <div className="text-right text-olive-700 dark:text-olive-700-dark"> + {BAR_SCALE}% higher + </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/sections/jobs-monitor.tsx around lines 107 - 120, The desktop legend is missing the positive-end scale label; update the last empty <div> in jobs-monitor.tsx's legend grid (the same block using BAR_SCALE) to display the max positive label (e.g., "{BAR_SCALE}% higher") so the five columns read symmetrically: "{BAR_SCALE}% lower", "{BAR_SCALE / 2}% lower", "2025 Baseline", "{BAR_SCALE / 2}% higher", "{BAR_SCALE}% higher". Ensure you keep the existing text color classes consistent with the other positive labels (text-olive-700 and dark:text-olive-700-dark).
🧹 Nitpick comments (28)
front_end/src/components/ui/tailwind-indicator.tsx (1)
5-5: Make the debug badge non-interactive to avoid blocking clicks.On Line 5, the fixed badge can cover actionable UI in the bottom-right during development. Consider disabling pointer interaction and hiding it from assistive tech.
Suggested tweak
- <div className="fixed bottom-1 right-1 z-[10000] flex h-6 w-6 items-center justify-center rounded-full bg-black p-3 font-mono text-[10px] font-semibold text-white"> + <div + aria-hidden="true" + className="pointer-events-none fixed bottom-1 right-1 z-[10000] flex h-6 w-6 items-center justify-center rounded-full bg-black p-3 font-mono text-[10px] font-semibold text-white" + >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/components/ui/tailwind-indicator.tsx` at line 5, The fixed debug badge is currently intercepting clicks; in the TailwindIndicator component update the root div (the element with className starting "fixed bottom-1 right-1 ...") to be non-interactive and hidden from assistive tech by adding the "pointer-events-none" utility to its className and adding aria-hidden="true" (or role="presentation") to the element so it won't block pointer events or be exposed to screen readers.front_end/src/app/(main)/labor-hub/helpers/labor-hub-format.ts (1)
2-4: Hardcoded English strings "shrink"/"grow" may hinder localization.The function returns hardcoded English words. If the Labor Hub is intended for localized audiences, consider accepting a translation function or returning keys that can be localized at the call site. Based on learnings: "Do not hardcode English strings in TSX components... Prefer using useTranslations() and i18n strings for UI text."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/helpers/labor-hub-format.ts around lines 2 - 4, formatOccupationChange currently returns hardcoded English words ("shrink"/"grow"), which blocks localization; update the function signature to accept either a translation callback (e.g., t: (key: string) => string) or return a structured object/enum (e.g., { direction: 'shrink'|'grow', percent: string } or keys like "occupation.shrink"/"occupation.grow") so callers can localize text with useTranslations; modify the implementation in formatOccupationChange to use the passed translator or to emit localization keys instead of literal strings while keeping the numeric formatting Math.abs(value).toFixed(0) intact and update any callers of formatOccupationChange accordingly.front_end/src/app/(main)/labor-hub/components/question-cards/placeholder.tsx (1)
27-29: Hardcoded English string should use i18n.The text "Question not available yet" is hardcoded in English. Consider using
useTranslations()from the app's i18n setup for proper localization. Based on learnings: "Do not hardcode English strings in TSX components... Prefer using useTranslations() and i18n strings for UI text."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question-cards/placeholder.tsx around lines 27 - 29, Replace the hardcoded English string in the Placeholder component with an i18n lookup: import and call useTranslations() (e.g., const t = useTranslations('laborHub')) in the component and replace "Question not available yet" with t('questionNotAvailable') (or an appropriate key like 'placeholder.questionNotAvailable') so the UI text is localized; add the corresponding key/value to the i18n namespace used by useTranslations.front_end/src/app/(main)/labor-hub/data.ts (1)
1-84: Consider adding TypeScript types for data structures.Adding explicit type definitions for
JOBS_DATAandMETHODOLOGY_SECTIONSwould improve maintainability and catch errors at compile time when modifying data.♻️ Proposed type definitions
+type GovernmentBaselines = Record<string, number>; + +type JobEntry = { + name: string; + post_id: number; + rating: number; +}; + +type FAQ = { + question: string; + answer: string; +}; + +type MethodologySection = { + title: string; + faqs: FAQ[]; +}; + -export const GOVERNMENT_BASELINES = { +export const GOVERNMENT_BASELINES: GovernmentBaselines = { "2025": 0, ... }; -export const JOBS_DATA = [ +export const JOBS_DATA: JobEntry[] = [ ... ]; -export const METHODOLOGY_SECTIONS = [ +export const METHODOLOGY_SECTIONS: MethodologySection[] = [ ... ];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/data.ts around lines 1 - 84, Add explicit TypeScript types and annotate the exported constants: define an interface (e.g., JobEntry) for JOBS_DATA items with fields name: string, post_id: number, rating: number, and type for GOVERNMENT_BASELINES (e.g., Record<string, number> or a typed BaselineMap for known years); then update the exported declarations to use those types (export const JOBS_DATA: JobEntry[] = ... and export const GOVERNMENT_BASELINES: Record<string, number> = ...). Also add and use a METHODOLOGY_SECTIONS type (e.g., Section[]) if that structure exists elsewhere, and export the types so callers can rely on them for compile-time checks.front_end/src/app/(main)/labor-hub/components/print-attribution.tsx (1)
12-16: Hardcoded"en-US"locale may hinder localization.The date format uses a hardcoded English locale. Consider using the user's locale or leveraging the app's i18n setup for consistent localization across the application. Based on learnings: "Do not hardcode English strings in TSX components... Prefer using useTranslations() and i18n strings for UI text."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/print-attribution.tsx around lines 12 - 16, The date rendering currently calls new Date().toLocaleDateString("en-US", ...) which hardcodes the locale; update the PrintAttribution component to use the app's localization instead—replace the fixed "en-US" with the current locale from your i18n provider (e.g., useTranslations()/useLocale() or the app-level locale prop) and/or format the date via the i18n/date utility so it respects user locale and translation settings; locate the new Date().toLocaleDateString(...) call in print-attribution.tsx and wire it to the app's locale hook or formatting helper instead of the literal "en-US".front_end/src/app/(main)/labor-hub/components/activity-card.tsx (1)
61-61: Hardcoded Englisharia-labelshould use i18n.The
aria-label="Open external link"is hardcoded in English. For accessibility in localized contexts, consider using a translated string. Based on learnings: "Do not hardcode English strings in TSX components... Prefer using useTranslations() and i18n strings for UI text."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/activity-card.tsx at line 61, The aria-label "Open external link" in activity-card.tsx is hardcoded; update the ActivityCard component to use useTranslations() instead: import and call useTranslations (e.g., const t = useTranslations('ActivityCard')) and replace aria-label="Open external link" with aria-label={t('openExternalLink')}; add the corresponding "openExternalLink" entry to the ActivityCard i18n namespace/locale files so translations are available.front_end/src/app/(main)/labor-hub/sections/hero.tsx (1)
19-28: Hardcoded English strings in hero section should use i18n.The title ("Labor Automation Forecasting Hub") and description text are hardcoded in English. Consider using
useTranslations()for these user-facing strings to support localization. Based on learnings: "Do not hardcode English strings in TSX components... Prefer using useTranslations() and i18n strings for UI text."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/sections/hero.tsx around lines 19 - 28, Replace the hardcoded English strings in the hero TSX (the h1 text "Labor Automation Forecasting Hub" and the p description) with localized values using useTranslations(): import useTranslations and call it (e.g. const t = useTranslations('laborHub') or your chosen namespace), then replace the literal texts in the h1 and p with t('title') and t('description') (or your chosen keys), and add those keys and translations to the i18n message files for the namespace so the component reads strings from i18n instead of hardcoded English.front_end/src/app/(main)/labor-hub/components/question-cards/question-card.tsx (4)
303-309: Toast messages should use i18n.The success message
"Image downloaded successfully"and error message"Failed to export image"are hardcoded English strings. Based on learnings, these should use translated strings viauseTranslations().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question-cards/question-card.tsx around lines 303 - 309, Replace the hardcoded toast strings in question-card.tsx with i18n translations: import and call useTranslations (or use the existing t from useTranslations in the QuestionCard component) and replace "Image downloaded successfully" and "Failed to export image" with appropriate translation keys (e.g. t('image.download_success') and t('image.export_failed')) when calling toast(...) and toast.error(...); ensure the translation keys are added to your locale files and that the component has access to the t function (e.g., via const t = useTranslations('namespace')) so the toast messages use translated text.
385-389: Footer attribution text should use i18n.The strings
"Data from"and"on"are hardcoded. These should be translated for localization support. Based on learnings, avoid hardcoded English strings in TSX components.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question-cards/question-card.tsx around lines 385 - 389, Replace the hardcoded English footer text with i18n keys: instead of literal "Data from" and "on", use the translation function (e.g., t or useTranslations) used across the app and compose the string around MetaculusTextLogo and formatCurrentDate(); update the component (question-card.tsx) to import and call the translator (e.g., const t = useTranslations('...') or t('laborHub.footer')) and use t('dataFrom') and t('onDate') (or a single key like t('dataFromOn', { source: 'Metaculus', date: formatCurrentDate() })) so the UI text is localizable while keeping MetaculusTextLogo and formatCurrentDate() unchanged.
21-27: Hardcoded locale in date formatting.The
formatCurrentDate()function hardcodes"en-US"locale. Consider using the app's locale context or a formatting utility that respects user preferences for consistency with the rest of the application.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question-cards/question-card.tsx around lines 21 - 27, The formatCurrentDate function currently hardcodes the "en-US" locale; update it to use the application's locale provider instead (e.g. read locale from your i18n/context hook or prop) so date formatting respects user preferences. Locate formatCurrentDate and replace the hardcoded locale with the app's locale value (or fall back to undefined to use the runtime locale), using Intl.DateTimeFormat or toLocaleDateString with that dynamic locale; ensure any imports or hooks (e.g. useLocale, i18n.locale) are referenced and passed into where formatCurrentDate is called if it cannot access context directly.
121-186: Hardcoded English strings should use i18n.Labels like
"View Question","View Questions","Export PNG","Export CSV", and"Share on X"are hardcoded. Based on learnings, prefer usinguseTranslations()and i18n strings for UI text across components to avoid stalling localization.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question-cards/question-card.tsx around lines 121 - 186, Replace the hardcoded UI strings in the moreMenuItems array with i18n calls: import and call useTranslations (e.g., const t = useTranslations('...')) at the top of the component and substitute labels like "View Question"/"View Questions", "Export PNG", "Export CSV", and "Share on X" with t('viewQuestion')/t('viewQuestions') (or a pluralized t usage keyed appropriately), t('exportPng'), t('exportCsv'), and t('shareOnX') respectively in the MenuLinkWithIcon and MenuItemWithIcon elements; ensure you add the corresponding translation keys to your locale files and handle the hasMultiplePosts conditional by using separate keys or i18n pluralization.front_end/src/app/(main)/labor-hub/components/table-compact.tsx (1)
73-82: Currency and unit formatting should consider i18n.The
WageValuecomponent hardcodes$and/hr. For localization support, consider usingIntl.NumberFormatfor currency formatting and translated unit strings. However, if this Labor Hub feature is US-specific, this may be acceptable. Based on learnings about i18n, prefer translated strings for UI text.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/table-compact.tsx around lines 73 - 82, WageValue currently hardcodes "$" and "/hr"; update the WageValue component to use Intl.NumberFormat to format the numeric value (use navigator.language or accept a locale/currency prop) and replace the hardcoded "/hr" with a translatable unit string (pull from your i18n function/namespace or accept a unit prop); ensure the component signature (WageValue props: value, className) is extended to accept optional locale/currency/unit or to read them from context so formattedCurrency = new Intl.NumberFormat(locale, { style: 'currency', currency }).format(value) is used and the unit string comes from your translation helper instead of a literal.front_end/src/app/(main)/labor-hub/components/question-cards/flippable-question-card.tsx (1)
57-58: Aria-labels should use i18n.The
aria-labelattributes"Show first view"and"Show second view"are hardcoded English strings. For accessibility in localized contexts, these should use translated strings. Based on learnings, prefer usinguseTranslations()for UI text.Also applies to: 73-74
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question-cards/flippable-question-card.tsx around lines 57 - 58, Replace the hardcoded aria-label values in the flippable-question-card component with localized strings: import and call useTranslations() (e.g., const t = useTranslations()), add translation keys for the two labels (e.g., "flippable.showFirstView" and "flippable.showSecondView"), and use t(...) for the aria-label props on the buttons that currently use aria-label="Show first view" / "Show second view" (the elements that also use aria-pressed and selected === "left"). Ensure the new keys are added to the locale JSONs so the buttons render localized aria-labels.front_end/src/app/(main)/labor-hub/components/mobile-carousel.tsx (2)
43-43: Unnecessary empty string incn().The empty string
""incn("", className)serves no purpose and can be simplified.Proposed fix
- <div className={cn("", className)}> + <div className={className}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/mobile-carousel.tsx at line 43, Remove the unnecessary empty string passed to the classnames helper: update the JSX in the MobileCarousel component where it calls cn("", className) to call cn(className) (or simply use className directly if no other classes are combined); modify the element rendering in mobile-carousel.tsx that uses cn to remove the redundant "" argument so the class composition uses only the meaningful className value.
59-59: Consider using i18n for aria-label.The aria-label
"Go to slide ${i + 1}"is hardcoded in English. For full localization support, this should use a translation function.Based on learnings: "Do not hardcode English strings in TSX components."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/mobile-carousel.tsx at line 59, Replace the hardcoded aria-label in MobileCarousel's JSX (aria-label={`Go to slide ${i + 1}`}) with a call to the app's i18n translation function (e.g., t or translate) using a key like "carousel.goToSlide" and pass the slide index as an interpolation value; update the i18n resource files (all locales) to include "carousel.goToSlide": "Go to slide {{number}}" (and localized strings) so the aria-label becomes t('carousel.goToSlide', { number: i + 1 }) instead of the literal English string.front_end/src/app/(main)/labor-hub/sections/engagement-section.tsx (2)
67-73: ShareCard also has hardcoded strings."Share the dashboard", "Think this could help others...", "Copy Link", and "Share on X" should be translated using
useTranslations().Based on learnings: "Do not hardcode English strings in TSX components."
Also applies to: 81-92
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/sections/engagement-section.tsx around lines 67 - 73, In the ShareCard within engagement-section.tsx, replace the hardcoded English UI strings ("Share the dashboard", the paragraph starting "Think this could help others...", "Copy Link", and "Share on X") with translations from useTranslations(); import and call useTranslations() at the top of the component, use a unique namespace or keys (e.g., t('share.title'), t('share.description'), t('share.copyButton'), t('share.xButton')) when rendering the h3, paragraph and buttons, and ensure corresponding keys are added to the i18n message files so the component no longer contains hardcoded English; also update lines 81-92 where the other hardcoded strings appear to use the same t(...) keys.
26-47: Hardcoded English strings should use i18n.Multiple UI strings are hardcoded in English: "Reach out to Metaculus", "Have thoughts, questions...", "View Services", "Contact Us". These should use
useTranslations()for localization support.Based on learnings: "Do not hardcode English strings in TSX components... Prefer using useTranslations() and i18n strings for UI text across components."
Example refactor for PartnerCard
+import { useTranslations } from "next-intl"; + function PartnerCard() { const { setCurrentModal } = useModal(); + const t = useTranslations(); return ( <div className="..."> ... - <h3 className="..."> - Reach out to Metaculus - </h3> + <h3 className="..."> + {t("laborHub.reachOut")} + </h3> ... </div> ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/sections/engagement-section.tsx around lines 26 - 47, Replace the four hardcoded UI strings in engagement-section.tsx with i18n translation calls: import and call useTranslations (e.g., const t = useTranslations('engagement')) at the top of the component, then replace "Reach out to Metaculus", the paragraph text, "View Services", and "Contact Us" with t('reachOutTitle'), t('description'), t('viewServices'), and t('contactUs') respectively (or similarly named keys); ensure setCurrentModal usage and Link/button props remain unchanged. Add these keys and translated values to the relevant locale JSON files under the same namespace ('engagement') so translations load correctly. Ensure no logic or styling changes are made, only string replacements to use t(...) calls.front_end/src/app/(main)/actions.ts (1)
21-24: UnknownlistKeysilently falls back to default list.If a caller passes an invalid
listKey(e.g., a typo like"Labour"),NEWSLETTER_LIST_IDS[listKey]returnsundefined, and the subscription silently goes to the default list instead of the intended one. Consider whether this should log a warning or throw an error for easier debugging.Optional: add a warning for unknown keys
export async function subscribeToNewsletter(email: string, listKey?: string) { - const listId = listKey ? NEWSLETTER_LIST_IDS[listKey] : undefined; + let listId: string | undefined; + if (listKey) { + listId = NEWSLETTER_LIST_IDS[listKey]; + if (!listId) { + console.warn(`Unknown newsletter listKey: "${listKey}", using default list`); + } + } return await serverMiscApi.subscribeToNewsletter(email, listId); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/actions.ts around lines 21 - 24, The function subscribeToNewsletter quietly treats an invalid listKey as undefined and falls back to the default list; update subscribeToNewsletter to validate the provided listKey against NEWSLETTER_LIST_IDS: if listKey is provided but not a key in NEWSLETTER_LIST_IDS, either throw an error or log a warning (choose one consistent with app behavior) and do not call serverMiscApi.subscribeToNewsletter with a wrong listId; reference the symbols subscribeToNewsletter, NEWSLETTER_LIST_IDS, and serverMiscApi.subscribeToNewsletter when implementing the check and the warning/throw.front_end/src/app/(main)/labor-hub/components/question-cards/flippable-multi-question-card.tsx (1)
50-56: Props spread order may cause unexpected overrides.The
{...tableProps}spread comes before explicit props liketitleandrows, which is correct — explicit props will override any conflicting tableProps. However,historicalValueKeysuses a differently-named source prop (tableHistoricalValueKeys), which could be confusing for maintainers.Consider renaming
tableHistoricalValueKeysto match the target prop name or adding a comment explaining the mapping.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question-cards/flippable-multi-question-card.tsx around lines 50 - 56, The MultiQuestionTable usage spreads {...tableProps} and then passes explicit props title, rows and historicalValueKeys, but the source prop is named tableHistoricalValueKeys which is confusing; either rename tableHistoricalValueKeys to historicalValueKeys (so it directly maps and avoids mental mapping) or add a concise inline comment next to the MultiQuestionTable JSX explaining that tableHistoricalValueKeys intentionally maps to historicalValueKeys to prevent surprises when overriding props from tableProps (refer to MultiQuestionTable, tableProps, tableHistoricalValueKeys, historicalValueKeys, title, rows).front_end/src/app/(main)/labor-hub/page.tsx (2)
308-309: TODO comment indicates potentially incorrect link.The comment notes this "can be wrong link for demo." Verify the link
/questions/42220/change-in-us-bachelors-relative-to-2024-25/#comment-793578is correct or remove/update it before release.Would you like me to open an issue to track this link verification?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/page.tsx around lines 308 - 309, The hard-coded demo link assigned to the variable/name "link" in page.tsx may be incorrect; verify the target URL "/questions/42220/change-in-us-bachelors-relative-to-2024-25/#comment-793578" points to the intended original comment and either update it to the correct URL or remove the demo link and replace with a dynamic/placeholder implementation (e.g., derive from the comment ID or use a configurable constant) in the component that renders the comment link so the production build does not ship a possibly wrong static link.
28-37: Tab labels should use i18n.The
TABSarray contains hardcoded English labels. Per learnings, these should use translation keys viauseTranslations()to support localization.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/page.tsx around lines 28 - 37, TABS currently contains hardcoded English labels; update page.tsx to build the TABS array using the i18n hook by importing and calling useTranslations() inside the component (or page) and replacing each label with t("...") translation keys (e.g., t("laborHub.tabs.overview") etc.), ensuring the unique identifiers (TABS, useTranslations) are used so the UI renders localized strings and translation keys are added to your locale files.front_end/src/app/(main)/labor-hub/components/newsletter-subscribe-popover.tsx (2)
77-84: Additional hardcoded English strings.The title "Labor Hub Newsletter" and description text should use i18n. Per learnings, prefer
useTranslations()for UI text.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/newsletter-subscribe-popover.tsx around lines 77 - 84, The component contains hardcoded UI strings "Labor Hub Newsletter" and the description; replace them with i18n using useTranslations(): import and call useTranslations() in the newsletter-subscribe-popover component (front_end/src/app/(main)/labor-hub/components/newsletter-subscribe-popover.tsx), add translation keys like laborHub.newsletter.title and laborHub.newsletter.description (or similar names) and replace the JSX text nodes with t('laborHub.newsletter.title') and t('laborHub.newsletter.description'); ensure the useTranslations() call is placed at the top of the component function and update the translation resource files accordingly.
68-72: Hardcoded success message mentions specific newsletter name.The success message "Labor Hub Newsletter" is hardcoded. If this component is meant to be reusable for other newsletters (since it accepts
listKey), the message should be dynamic or use i18n with interpolation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/newsletter-subscribe-popover.tsx around lines 68 - 72, The success message in the NewsletterSubscribePopover component is hardcoded to "Labor Hub Newsletter"; change it to render a dynamic newsletter name using the component prop (listKey) or an i18n interpolation key instead—locate the JSX paragraph in NewsletterSubscribePopover and replace the literal "Labor Hub Newsletter" with either a derived display name from listKey (e.g., map or formatListName(listKey)) or use your translation helper (e.g., t('newsletter.subscribed', { name: listName })) so the message is reusable for other lists.utils/views.py (1)
136-143: Consider consistent type coercion forpost_ids.For GET requests,
getlistreturns strings, while for POST requests with JSON body,post_idsmay already be integers. Line 143's comparisonstr(pid) != str(post_id)handles this, but it would be cleaner to normalize all IDs to integers early.♻️ Suggested normalization
# Handle multiple post_ids # For GET requests, use getlist to handle repeated query params (post_ids=1&post_ids=2) if request.method == "GET": post_ids = request.GET.getlist("post_ids") or [] else: post_ids = data.get("post_ids") or [] + # Normalize to integers for consistent comparison + post_ids = [int(pid) for pid in post_ids] if post_id := data.get("post_id"): - post_ids = [post_id] + [pid for pid in post_ids if str(pid) != str(post_id)] + post_id = int(post_id) + post_ids = [post_id] + [pid for pid in post_ids if pid != post_id]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@utils/views.py` around lines 136 - 143, Normalize post_ids to integers early in the request handling block in utils/views.py: when building post_ids in the GET branch (where request.GET.getlist("post_ids") yields strings) and in the else branch (data.get("post_ids")), coerce all entries to ints (safely ignoring or logging non-numeric values) and also cast data.get("post_id") to int before merging; update the merging logic around the post_id check so it compares integers rather than stringified values (refer to the GET/else post_ids assignment and the post_id merge block).front_end/src/app/(main)/labor-hub/components/question-cards/question.tsx (1)
152-156: Duplicated note rendering logic.The note rendering block is identical in both the flippable and non-flippable code paths. Consider extracting to a small helper or shared fragment.
♻️ Suggested extraction
+const NoteBlock = ({ note }: { note?: ReactNode }) => + note ? ( + <div className="!mt-2 text-sm text-blue-700 dark:text-blue-700-dark"> + {note} + </div> + ) : null; // Then in both render paths: -{note && ( - <div className="!mt-2 text-sm text-blue-700 dark:text-blue-700-dark"> - {note} - </div> -)} +<NoteBlock note={note} />Also applies to: 177-181
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question-cards/question.tsx around lines 152 - 156, Extract the duplicated note rendering JSX (the conditional block that checks "note" and renders <div className="!mt-2 text-sm text-blue-700 dark:text-blue-700-dark">{note}</div>) into a small shared helper or child component (e.g., NoteFragment or renderNote) and replace the two occurrences in the flippable and non-flippable branches of the Question component in question.tsx with that helper; ensure the helper accepts the note string (and any className overrides if needed) and is imported/declared near the Question component so both code paths call the same symbol.front_end/src/app/(main)/labor-hub/sections/activity-monitor.tsx (1)
27-87: Hardcoded English content strings.All activity entries contain hardcoded English text. Per learnings, UI text should use i18n strings via
useTranslations()to support localization.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/sections/activity-monitor.tsx around lines 27 - 87, The activity entries in activity-monitor.tsx contain hardcoded English JSX strings (entries with ids like "gpt-5-release", "creative-roles-insight", "amazon-workforce-cuts", "developer-projection-falls"); replace those hardcoded texts with i18n lookups using useTranslations() (import and call useTranslations at the top of the component), move each human-facing string into translation keys (e.g., activity.gpt5.title, activity.gpt5.body, activity.amazon.source) and use t('...') (with interpolation for variables like percentages or source URLs) inside the content JSX for each entry; ensure links, strong/em/emphasis segments and punctuation are represented by translated fragments or by splitting static translated strings and JSX wrappers so the rendered markup and translations remain correct.front_end/src/app/(main)/labor-hub/sections/research.tsx (1)
24-63: Hardcoded English strings should use i18n.Multiple UI strings are hardcoded in English (section header, content paragraphs, activity card text). Based on learnings, prefer using
useTranslations()and i18n strings for UI text to avoid stalling localization.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/sections/research.tsx around lines 24 - 63, This file contains multiple hardcoded English UI strings (SectionHeader text, ContentParagraph blocks, ActivityCard children and props) — import and call useTranslations() at the top of the component and replace each literal with translation calls (e.g. t('laborHub.research.header'), t('laborHub.research.paragraph1'), t('laborHub.research.paragraph2'), t('laborHub.research.activity.text'), and keys for ActivityCard props like t('laborHub.research.activity.username')/t('laborHub.research.activity.subtitle')) and add corresponding keys/strings to the i18n resource file; ensure SectionHeader, ContentParagraph, QuestionLoader and ActivityCard usages pass translated strings (including children) instead of hardcoded text.front_end/src/app/(main)/labor-hub/components/question-cards/multi-question-data.ts (1)
34-41: Resolve shared datasets once upstream.
fetchMultiQuestionDataset()always performs a fresh posts fetch. In this PR the same row config is used by multiple question-card presentations, so rendering both views can fan out into identical API calls and mapping work. Consider loading the dataset once in a parent/server wrapper and passing the resolved result down.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question-cards/multi-question-data.ts around lines 34 - 41, fetchMultiQuestionDataset always refetches posts for the given MultiQuestionRowConfig list, causing duplicate identical API calls when the same config is used across multiple question-card presentations; refactor to accept an optional preloaded dataset parameter or move the fetch to a higher-level loader and reuse it: update fetchMultiQuestionDataset (and its callers) so it can take a resolved MultiQuestionDataset (or be replaced by a parent/server wrapper that calls ServerPostsApi.getPostsWithCP once for the postIds and passes the results down), and ensure code paths that currently call ServerPostsApi.getPostsWithCP({ ids: postIds, limit: postIds.length }) are consolidated to use the shared result instead of calling the API repeatedly.
| <> | ||
| <div | ||
| className={cn( | ||
| useMobileCarousel ? "hidden lg:flex" : "flex", | ||
| "min-w-0 flex-col space-y-6 print:flex print:space-y-4 [&>*]:min-w-0", | ||
| className | ||
| )} | ||
| {...props} | ||
| > | ||
| {children} | ||
| </div> | ||
| {useMobileCarousel && ( | ||
| <div className="-mx-5 md:-mx-10 lg:hidden print:hidden"> | ||
| <MobileCarousel>{children}</MobileCarousel> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
Avoid mounting the right-pane children twice.
When useMobileCarousel is enabled, this renders the same children once in the hidden desktop container and again inside MobileCarousel. For charts and other stateful content, that means duplicate IDs, duplicate effects, and roughly double render work even though only one branch is visible. Please render a single branch, or have callers provide separate mobile/desktop presenters instead of reusing the same subtree twice.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@front_end/src/app/`(main)/labor-hub/components/section.tsx around lines 86 -
101, The current implementation mounts the same children twice when
useMobileCarousel is true (once in the hidden-for-mobile desktop container and
again inside MobileCarousel); update the JSX so only one branch mounts the
children: render the desktop wrapper div only when !useMobileCarousel and render
the MobileCarousel only when useMobileCarousel, keeping the existing className,
{...props}, and children placement intact; reference the useMobileCarousel prop,
the desktop wrapper div that currently contains {children}, and the
MobileCarousel component to locate where to change the conditional rendering.
There was a problem hiding this comment.
🧹 Nitpick comments (5)
front_end/src/app/(main)/labor-hub/components/question-cards/question-card.tsx (5)
232-249: Unnecessary fragment wrapper.The
<>...</>fragment wrapper aroundDropdownMenuis not needed since there's only one child element.Remove unnecessary fragment
return ( - <> - <DropdownMenu - items={moreMenuItems} - textAlign="right" - className="min-w-36 overflow-hidden rounded-md border-blue-400 bg-gray-0 dark:border-blue-400-dark dark:bg-gray-0-dark" - > - <Button - aria-label="more options" - size="md" - presentationType="icon" - className="border-transparent bg-transparent text-blue-700 hover:!bg-blue-400 group-hover/card:bg-blue-200 data-[open]:!bg-blue-700 data-[open]:!text-gray-0 dark:border-transparent dark:bg-transparent dark:text-blue-700-dark dark:hover:!bg-blue-400-dark dark:group-hover/card:bg-blue-200-dark dark:data-[open]:!bg-blue-700-dark dark:data-[open]:!text-gray-0-dark" - > - <FontAwesomeIcon icon={faEllipsis} /> - </Button> - </DropdownMenu> - </> + <DropdownMenu + items={moreMenuItems} + textAlign="right" + className="min-w-36 overflow-hidden rounded-md border-blue-400 bg-gray-0 dark:border-blue-400-dark dark:bg-gray-0-dark" + > + <Button + aria-label="more options" + size="md" + presentationType="icon" + className="border-transparent bg-transparent text-blue-700 hover:!bg-blue-400 group-hover/card:bg-blue-200 data-[open]:!bg-blue-700 data-[open]:!text-gray-0 dark:border-transparent dark:bg-transparent dark:text-blue-700-dark dark:hover:!bg-blue-400-dark dark:group-hover/card:bg-blue-200-dark dark:data-[open]:!bg-blue-700-dark dark:data-[open]:!text-gray-0-dark" + > + <FontAwesomeIcon icon={faEllipsis} /> + </Button> + </DropdownMenu> );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question-cards/question-card.tsx around lines 232 - 249, Remove the unnecessary fragment wrapper surrounding the single JSX element by returning the DropdownMenu directly (no <>...</>), keeping the existing props and children (DropdownMenu, Button, FontAwesomeIcon with faEllipsis) intact; locate the return in the QuestionCard component (the block that returns the JSX containing DropdownMenu and Button) and replace the fragment-wrapped return with a direct return of the DropdownMenu element.
418-423: Footer attribution date is captured at render time, not export time.The
formatCurrentDate()is called during render, so if the card is rendered before midnight and exported after, the date shown in the PNG will be when the component rendered, not when the export occurred. If export-time accuracy matters, consider moving this to the export function.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question-cards/question-card.tsx around lines 418 - 423, The footer currently calls formatCurrentDate() during render inside QuestionCard (the JSX block with MetaculusTextLogo), causing the displayed date to reflect render time; move date generation out of render and into the export/png generation path so the export uses a fresh date at export-time (e.g., compute date in the export handler that triggers PNG creation and pass it into QuestionCard props or inject it into the DOM just before capture). Update usages of formatCurrentDate() in question-card.tsx accordingly so the render uses a prop like exportDate (or falls back to a cached renderDate) while the actual export flow supplies the correct export-time value.
23-29: Hardcoded locale and strings violate i18n best practices.The
formatCurrentDatefunction uses a hardcoded"en-US"locale. Additionally, this file contains multiple hardcoded English strings (toast messages, menu labels like "View Question", "Export PNG", "Export CSV", "Share on X", attribution text "Data from...on").Consider using
useTranslations()for UI text and respecting user locale preferences for date formatting. Based on learnings: "Do not hardcode English strings in TSX components... Prefer using useTranslations() and i18n strings for UI text."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question-cards/question-card.tsx around lines 23 - 29, The function formatCurrentDate and several UI strings in question-card.tsx are hardcoded to "en-US" and English text; replace the hardcoded locale with the runtime/user locale (e.g., use Intl.DateTimeFormat with a locale from next-intl or the user's browser/props) and move all user-facing strings (toast messages, menu labels "View Question", "Export PNG", "Export CSV", "Share on X", attribution text) into the i18n system via useTranslations() (or the project's translation hook), then read localized strings from that hook where the component renders; update formatCurrentDate to accept or obtain the locale from the same source so dates respect user preferences and swap all hardcoded English strings to translation keys used by useTranslations().
316-350: Avoidanytype and potential race condition in PNG export.
Line 341 uses
anytype for the catch parameter. Preferunknownfor better type safety.If
handleExportPngis called multiple times rapidly before the previous export completes, the CSS variable cleanup infinallycould interfere with an in-progress export. Consider adding a loading guard similar toisCsvDownloading.Proposed improvements
export function QuestionCard({ ... }) { const cardRef = useRef<HTMLDivElement>(null); + const [isPngExporting, setIsPngExporting] = useState(false); const titleVariant = titleVariantOverride ?? variant; const handleExportPng = useCallback(() => { - if (!cardRef.current) return; + if (!cardRef.current || isPngExporting) return; const node = cardRef.current; + setIsPngExporting(true); // Set CSS variables on the node before toPng clones it node.style.setProperty("--ss-visible", "visible"); node.style.setProperty("--ss-hidden", "hidden"); toPng(node, { pixelRatio: 2, style: { margin: "0px", borderRadius: "0px", }, }) .then((dataUrl: string) => { // ... }) - .catch((err: any) => { + .catch((err: unknown) => { console.error("Error exporting image:", err); toast.error("Failed to export image"); }) .finally(() => { // Clean up CSS variables after toPng completes node.style.removeProperty("--ss-visible"); node.style.removeProperty("--ss-hidden"); + setIsPngExporting(false); }); - }, [title]); + }, [title, isPngExporting]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question-cards/question-card.tsx around lines 316 - 350, Change the catch parameter in handleExportPng from any to unknown and narrow it before logging (e.g., check instanceof Error or stringify) to avoid using any; also prevent race conditions by introducing a guard (e.g., isPngDownloading state or pngExportInProgress ref) or export token: return early if already exporting, set the guard true before calling toPng and false in finally, or capture a unique exportId and only remove the CSS variables when the current exportId matches the one started by this invocation; reference handleExportPng, cardRef, and the new guard name (e.g., isPngDownloading or pngExportToken) so the cleanup no longer interferes with concurrent exports.
102-110: Array reference in dependency could cause unnecessary recalculations.If the parent component passes a new array reference on each render (e.g.,
postIds={[1, 2, 3]}), thisuseMemowill recalculate on every render. Consider documenting that callers should memoize thepostIdsprop, or use a stable serialization for the dependency.Example: Use serialized array as dependency
const viewQuestionsHref = useMemo(() => { if (hasMultiplePosts) { return `/questions?ids=${postIds.join(",")}`; } if (singlePostId) { return `/questions/${singlePostId}`; } return null; - }, [hasMultiplePosts, postIds, singlePostId]); + }, [hasMultiplePosts, postIds.join(","), singlePostId]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question-cards/question-card.tsx around lines 102 - 110, The useMemo for viewQuestionsHref (function/view variable viewQuestionsHref) currently depends on the array reference postIds which can trigger unnecessary recalculations if the parent passes a new array instance each render; change the dependency from the array reference to a stable serialization (e.g., postIds?.join(",") or JSON.stringify(postIds)) so the memo only invalidates when the actual contents change, and add a short prop note that callers should prefer memoizing postIds if they can; update the useMemo dependency array to use the chosen serialized value instead of postIds and ensure the code handles undefined/null postIds when serializing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@front_end/src/app/`(main)/labor-hub/components/question-cards/question-card.tsx:
- Around line 232-249: Remove the unnecessary fragment wrapper surrounding the
single JSX element by returning the DropdownMenu directly (no <>...</>), keeping
the existing props and children (DropdownMenu, Button, FontAwesomeIcon with
faEllipsis) intact; locate the return in the QuestionCard component (the block
that returns the JSX containing DropdownMenu and Button) and replace the
fragment-wrapped return with a direct return of the DropdownMenu element.
- Around line 418-423: The footer currently calls formatCurrentDate() during
render inside QuestionCard (the JSX block with MetaculusTextLogo), causing the
displayed date to reflect render time; move date generation out of render and
into the export/png generation path so the export uses a fresh date at
export-time (e.g., compute date in the export handler that triggers PNG creation
and pass it into QuestionCard props or inject it into the DOM just before
capture). Update usages of formatCurrentDate() in question-card.tsx accordingly
so the render uses a prop like exportDate (or falls back to a cached renderDate)
while the actual export flow supplies the correct export-time value.
- Around line 23-29: The function formatCurrentDate and several UI strings in
question-card.tsx are hardcoded to "en-US" and English text; replace the
hardcoded locale with the runtime/user locale (e.g., use Intl.DateTimeFormat
with a locale from next-intl or the user's browser/props) and move all
user-facing strings (toast messages, menu labels "View Question", "Export PNG",
"Export CSV", "Share on X", attribution text) into the i18n system via
useTranslations() (or the project's translation hook), then read localized
strings from that hook where the component renders; update formatCurrentDate to
accept or obtain the locale from the same source so dates respect user
preferences and swap all hardcoded English strings to translation keys used by
useTranslations().
- Around line 316-350: Change the catch parameter in handleExportPng from any to
unknown and narrow it before logging (e.g., check instanceof Error or stringify)
to avoid using any; also prevent race conditions by introducing a guard (e.g.,
isPngDownloading state or pngExportInProgress ref) or export token: return early
if already exporting, set the guard true before calling toPng and false in
finally, or capture a unique exportId and only remove the CSS variables when the
current exportId matches the one started by this invocation; reference
handleExportPng, cardRef, and the new guard name (e.g., isPngDownloading or
pngExportToken) so the cleanup no longer interferes with concurrent exports.
- Around line 102-110: The useMemo for viewQuestionsHref (function/view variable
viewQuestionsHref) currently depends on the array reference postIds which can
trigger unnecessary recalculations if the parent passes a new array instance
each render; change the dependency from the array reference to a stable
serialization (e.g., postIds?.join(",") or JSON.stringify(postIds)) so the memo
only invalidates when the actual contents change, and add a short prop note that
callers should prefer memoizing postIds if they can; update the useMemo
dependency array to use the chosen serialized value instead of postIds and
ensure the code handles undefined/null postIds when serializing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1cc57fe6-d0c0-4c61-aa94-e3718b9f6898
📒 Files selected for processing (2)
front_end/src/app/(main)/labor-hub/components/question-cards/question-card.tsxfront_end/src/components/charts/group_chart.tsx
✅ Files skipped from review due to trivial changes (1)
- front_end/src/components/charts/group_chart.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
front_end/src/app/(main)/labor-hub/sections/activity-monitor-interactive.tsx (1)
113-115: Avoid wiring the same scroll handler twice.The modal list already calls
updateModalScrollStatethroughonScroll, so the nativescrolllistener added in the effect just doubles the same state update on every scroll. Keeping one path here will avoid redundant renders in the hottest interaction path.Also applies to: 177-178
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/sections/activity-monitor-interactive.tsx around lines 113 - 115, The scroll handler updateModalScrollState is being wired twice: once via the modal list's onScroll prop and again using scrollElement.addEventListener("scroll", updateModalScrollState, ...), which causes redundant state updates and extra renders; remove the native addEventListener usage (and the corresponding removeEventListener) in the effect(s) around scrollElement to rely solely on the modal list's onScroll, and do the same cleanup for the other occurrence noted (around lines 177-178) so updateModalScrollState is only invoked once per scroll event.
🤖 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)/labor-hub/sections/activity-monitor-interactive.tsx:
- Around line 182-200: The two fade overlay divs (the aria-hidden absolute divs
controlled by modalScrollState.canScrollUp and modalScrollState.canScrollDown)
use hardcoded white RGBA stops; change them to derive colors from the modal
background/dark mode tokens (e.g., CSS variables or tailwind theme colors) so
the gradient matches light/dark themes—update the inline style for both overlays
to reference a CSS variable (or add theme-specific classes) that represents the
modal background (or transparentized modal-bg) and add a dark-mode variant with
appropriate RGBA values; keep the same gradient directions and opacity logic
tied to modalScrollState.
---
Nitpick comments:
In
`@front_end/src/app/`(main)/labor-hub/sections/activity-monitor-interactive.tsx:
- Around line 113-115: The scroll handler updateModalScrollState is being wired
twice: once via the modal list's onScroll prop and again using
scrollElement.addEventListener("scroll", updateModalScrollState, ...), which
causes redundant state updates and extra renders; remove the native
addEventListener usage (and the corresponding removeEventListener) in the
effect(s) around scrollElement to rely solely on the modal list's onScroll, and
do the same cleanup for the other occurrence noted (around lines 177-178) so
updateModalScrollState is only invoked once per scroll event.
🪄 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: b2244ade-28fc-48e6-b470-b49d3c7d390c
⛔ Files ignored due to path filters (1)
front_end/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
front_end/package.jsonfront_end/src/app/(main)/labor-hub/activity-data.tsxfront_end/src/app/(main)/labor-hub/sections/activity-monitor-interactive.tsxfront_end/src/app/(main)/labor-hub/sections/activity-monitor.tsx
✅ Files skipped from review due to trivial changes (2)
- front_end/package.json
- front_end/src/app/(main)/labor-hub/activity-data.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- front_end/src/app/(main)/labor-hub/sections/activity-monitor.tsx
| { id: "overview", label: "Overview" }, | ||
| { id: "jobs", label: "Jobs" }, | ||
| { id: "wages", label: "Wages" }, | ||
| { id: "education", label: "Education" }, | ||
| { id: "economy", label: "Economy" }, | ||
| { id: "research", label: "Research" }, | ||
| { id: "state-wa", label: "State" }, | ||
| { id: "methodology", label: "Methodology" }, |
There was a problem hiding this comment.
hardcoded english strings
There was a problem hiding this comment.
page is not translated. content is english only.
| "use client"; | ||
|
|
There was a problem hiding this comment.
I guess it duplicates logic from multiple_choice_chart.tsx and group_chart.tsx. And it would be greate to extract a shared base or extend the existing chart instead of forking.
There was a problem hiding this comment.
No they're fundamentally different. This shows multiple group questions each question as a line with group items in x-axis like fan chart. Also has some historic data as hardcoded data, or some hardcoded lines by itself. Presentation/design is different too. It's unrealistic to change question charts with this one without a deeper design pass.
| export function TabGroup({ | ||
| tabs, | ||
| className, | ||
| }: { | ||
| tabs: { id: string; label: string }[]; | ||
| className?: string; | ||
| }) { |
There was a problem hiding this comment.
Doesn't this just reimplement existing tab_bar form the ui folder? Maybe we can just add there optional scroll-spy mode?
There was a problem hiding this comment.
this is simple button wrapper in scrollspy container. doesnt contain any state logic instead of tab_bar (and design is different too). Probably bad naming here. Changed names.
| async subscribeToNewsletter(email: string, overrideListId?: string) { | ||
| const { auth, listId } = this.getMailjetConfig(); |
There was a problem hiding this comment.
It accepts client-supplied list ID with no allow-list – so caller can subscribe emails to any Mailjet list. I guess it's better to validate against an allow-list ({ "labor": LABOR_LIST_ID, ... }) inside the server action.
There was a problem hiding this comment.
Does exactly this in actual server action code, this is the internal api called by the action
| if (searchParams[POST_IDS_FILTER]) { | ||
| const idsParam = searchParams[POST_IDS_FILTER]; | ||
| if (Array.isArray(idsParam)) { | ||
| filters.ids = idsParam.map(Number).filter((id) => !isNaN(id)); | ||
| } else if (typeof idsParam === "string") { | ||
| // Handle comma-separated values or single value | ||
| filters.ids = idsParam | ||
| .split(",") | ||
| .map(Number) | ||
| .filter((id) => !isNaN(id)); | ||
| } | ||
| } |
There was a problem hiding this comment.
I believe when ?ids=… present, it suppresses for_main_feed, for_consumer_view, and default status filter.
/questions?ids=1,2,3 could expose drafts/private/non-consumer posts.
There was a problem hiding this comment.
I think this should be safe. When ids is present, it does bypass some of the default feed-scoping behavior, but it still goes through the same backend permission checks as the question detail page. In practice, that means it matches /questions/[id] visibility rather than exposing anything new. And if user knows a questions id and can access its detail page, it should be visible here too.
So ?ids=... should not reveal private, draft, or otherwise unreleased questions to users who could not already access that post directly. The backend still enforces post permissions and approved/public visibility, so I don’t think this creates a private visibility issue.
| const rect = (svg as SVGElement).getBoundingClientRect(); | ||
| const x = e.clientX - rect.left; | ||
| const y = e.clientY - rect.top; | ||
| <div className="relative h-full"> |
There was a problem hiding this comment.
doesn't this cause any regressions?
There was a problem hiding this comment.
it should be safe -- checked the output in many places tiles/detail pages/embeds etc.
| @@ -0,0 +1,217 @@ | |||
| "use client"; | |||
There was a problem hiding this comment.
kebab-case filenames; rest of the repo uses snake_case
There was a problem hiding this comment.
Actionable comments posted: 13
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
♻️ Duplicate comments (3)
front_end/src/app/(main)/labor-hub/components/newsletter_subscribe_card.tsx (1)
38-42:⚠️ Potential issue | 🟠 MajorLocalize the card copy and toast messages.
This component still hardcodes all of its user-facing strings, so the subscription flow won't translate with the rest of the Labor Hub UI. Please move the heading, description, success copy, aria label, placeholder, CTA, and toast messages into
useTranslations().Based on learnings: Do not hardcode English strings in TSX components. Prefer using
useTranslations()and i18n strings for UI text across components, and avoid hardcoded literals that could stall localization.Also applies to: 61-97
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/newsletter_subscribe_card.tsx around lines 38 - 42, The component newsletter_subscribe_card.tsx is using hardcoded English strings (heading, description, aria-label, placeholder, CTA, and toast messages in the try/catch) — replace them with values from useTranslations(): import and call useTranslations() at the top of the component, add keys for the heading, description, ariaLabel, placeholder, buttonLabel, toastSuccess, and toastError, then use those translation values in the JSX (e.g., input placeholder, aria-label, button text) and in the toast calls inside the subscribe handler (replace the literal "Subscribed successfully!" and "Failed to subscribe. Please try again." with t('...') tokens); ensure the translation keys are consistently named and used across the success path (toast) and error catch and update any other hardcoded strings in lines 61-97 similarly.front_end/src/app/(main)/labor-hub/page.tsx (1)
28-37:⚠️ Potential issue | 🟠 MajorMove page copy and tab labels into i18n keys.
This page is still shipping extensive hardcoded English (tabs, metadata text, section headers, narrative copy, chart titles). Please switch these literals to
useTranslations()-backed keys so localization can proceed consistently.Based on learnings: Do not hardcode English strings in TSX components; prefer
useTranslations()and i18n strings for UI text across components.Also applies to: 40-43, 56-57, 61-68, 82-84, 87-89, 139-140, 223-224, 283-285, 325-325, 380-380
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/page.tsx around lines 28 - 37, The TABS array in page.tsx (const TABS = [...]) and multiple hardcoded UI strings across this file must be replaced with i18n keys using useTranslations(); replace each literal (e.g., tab labels "Overview","Jobs","Wages", etc., and the other hardcoded metadata, headers, narrative copy, chart titles at the listed ranges) with calls to the translation function (e.g., const t = useTranslations('laborHub'); then use t('tabs.overview') or t('metadata.title')), add corresponding keys in the locale files, and update any JSX that currently renders those literals to call t(...) so all UI text in this component is localized.front_end/src/app/(main)/labor-hub/components/labor_hub_by_job_vulnerability_card.tsx (1)
85-112:⚠️ Potential issue | 🟠 MajorExtract narrative and accessibility strings to i18n interpolated keys.
The body sentence plus chip
title/aria-labelvalues are still hardcoded English. Please move them to translation keys with interpolation (jobName,year, formatted change).Based on learnings: Do not hardcode English strings in TSX components; prefer
useTranslations()and i18n strings for UI text across components.Also applies to: 142-143, 190-191
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/labor_hub_by_job_vulnerability_card.tsx around lines 85 - 112, The JSX contains hardcoded English strings for the narrative and chip titles/aria-labels (see mostVulnerable2035, leastVulnerable2035, formatOccupationChange, occupationForecastClassName) — replace these with i18n interpolated keys via useTranslations(): create translation keys for the whole sentence and for the chip title/aria-labels that accept jobName, year (2035) and the formatted change, then update the component to call t('key', { jobName, year: 2035, change: formatOccupationChange(...)}), and use those translated strings for both visible text and any title/aria-label attributes; apply the same extraction for the other occurrences noted (lines ~142-143 and ~190-191).
🟡 Minor comments (10)
front_end/src/app/(main)/labor-hub/components/mobile_carousel.tsx-59-59 (1)
59-59:⚠️ Potential issue | 🟡 MinorLocalize the dot button
aria-labelinstead of hardcoding English.Line 59 hardcodes English UI text and should use
useTranslations().🌐 Proposed fix
import useEmblaCarousel from "embla-carousel-react"; import React, { ReactNode, useCallback, useEffect, useState } from "react"; +import { useTranslations } from "next-intl"; @@ export function MobileCarousel({ children, className }: Props) { + const t = useTranslations(); const slides = React.Children.toArray(children); @@ - aria-label={`Go to slide ${i + 1}`} + aria-label={t("laborHub.mobileCarousel.goToSlide", { index: i + 1 })}Based on learnings: Do not hardcode English strings in TSX components; prefer
useTranslations()and i18n strings for UI text.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/mobile_carousel.tsx at line 59, The aria-label for the dot button in mobile_carousel.tsx is hardcoded in English; replace the literal template string aria-label={`Go to slide ${i + 1}`} with a localized string from useTranslations(): import and call useTranslations() in the component, add a new i18n key (e.g. "carousel.goToSlide") that accepts a slide number, and use t('carousel.goToSlide', { number: i + 1 }) (or equivalent) to produce the aria-label so it’s localized across languages.front_end/src/app/(main)/labor-hub/sections/jobs_monitor.tsx-206-220 (1)
206-220:⚠️ Potential issue | 🟡 MinorProps spread on wrong element.
The
...propsis spread on the inner styled<div>(line 218), but the outer wrapper<div>(line 207) has no way to receive additional props likeclassName. This may cause unexpected behavior if consumers passclassNameexpecting it to apply to the visible card.🛠️ Suggested fix
export function InsightCard({ context, + className, ...props -}: { context: "positive" | "negative" } & ComponentProps<"div">) { +}: { context: "positive" | "negative"; className?: string } & ComponentProps<"div">) { return ( - <div className="pointer-events-auto relative bg-gray-0 dark:bg-gray-0-dark"> + <div className={cn("pointer-events-auto relative bg-gray-0 dark:bg-gray-0-dark", className)}> <div className={cn( ... )} {...props} /> </div> ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/sections/jobs_monitor.tsx around lines 206 - 220, The props spread is currently applied to the inner styled <div> (the one using the cn(...) call) so external props like className won't reach the outer wrapper; remove {...props} from the inner element and spread props on the outer wrapper <div> instead, and to preserve external className styling merge any incoming className into the cn(...) call (e.g., take props.className or destructure { className, ...rest } and use rest on the outer div and include className in the cn(...) for the inner div) so consumers' className and other props apply as expected.front_end/src/app/(main)/labor-hub/components/newsletter_subscribe_popover.tsx-53-60 (1)
53-60:⚠️ Potential issue | 🟡 MinorHide the close control when
onCloseisn't provided.
onCloseis optional, but the button is always focusable and announced as a close action. Without a handler, it becomes a dead control.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/newsletter_subscribe_popover.tsx around lines 53 - 60, The close button in the NewsletterSubscribePopover component (the JSX button using onClose and FontAwesomeIcon/faXmark) is always rendered and focusable even though onClose is optional; change the JSX so the button is only rendered when the onClose prop is provided (or alternatively make it non-focusable and aria-hidden and disabled when onClose is undefined). Locate the button that references onClose and wrap it in a conditional render (e.g., {onClose && (<button ... onClick={onClose}>...</button>)}) or set tabIndex={-1} aria-hidden="true" and disabled when onClose is falsy, ensuring no dead interactive control remains.front_end/src/app/(main)/labor-hub/activity_data.tsx-37-41 (1)
37-41:⚠️ Potential issue | 🟡 MinorReplace the placeholder Reuters citation before shipping.
href="#"renders a dead source link that just jumps to the top of the page, and the sentence already has a dangling closing quote. Either wire up the real Reuters URL or render plain text until the source exists.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/activity_data.tsx around lines 37 - 41, The JSX in activity_data.tsx currently uses an anchor <a href="#" className="underline hover:no-underline">Reuters</a> which creates a dead link and the sentence has a dangling closing quote after "AI." — replace the placeholder href="#" with the actual Reuters article URL if available (keeping the existing className), or remove the <a> and render "Reuters" as plain text; also remove the extraneous closing quote in the surrounding string so the text reads correctly. Target the JSX block around the existing anchor element in the activity component to apply this change.front_end/src/app/(main)/labor-hub/components/print_attribution.tsx-4-16 (1)
4-16:⚠️ Potential issue | 🟡 MinorRender the exact page URL with a locale-aware date.
window.location.hostname + window.location.pathnamedrops the protocol and any query/hash state, and"en-US"forces English month names in every locale. Prefer the full page URL plus the active app locale in this footer.Based on learnings: Do not hardcode English strings in TSX components. Prefer using
useTranslations()and i18n strings for UI text across components, and avoid hardcoded literals that could stall localization.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/print_attribution.tsx around lines 4 - 16, The current code builds a partial URL with window.location.hostname + window.location.pathname and hardcodes "en-US" for date formatting; update the component in print_attribution.tsx to use the full page URL (window.location.href when window is defined) and format the date with the active locale instead of "en-US". Wire in the app localization hooks (useTranslations() and the app locale source you use—e.g., useRouter().locale or your i18n locale getter) to obtain the current locale and pass that to toLocaleDateString, and replace any hardcoded UI strings with translated text from useTranslations() so the footer is localized. Ensure the const url variable and the date formatting call are updated accordingly and that the component falls back safely when window is undefined.front_end/src/app/(main)/labor-hub/page.tsx-424-427 (1)
424-427:⚠️ Potential issue | 🟡 MinorReplace placeholder forecaster identity and quote.
The “John Doe” + placeholder text is still visible in user-facing content and should be replaced with final source-backed copy.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/page.tsx around lines 424 - 427, The ActivityCard instance in page.tsx currently uses placeholder props (username="John Doe", subtitle="Pro Forecaster") and placeholder child text; replace these with the final forecaster identity and a source-backed quote: set username to the real forecaster name, subtitle to their official title (e.g., "Senior Labor Economist"), and replace the inner placeholder copy with the approved quote text and a short citation or source attribution. Ensure you only change the ActivityCard JSX props/children (the ActivityCard component is the unique symbol to edit) and preserve existing markup/props structure.front_end/src/app/(main)/labor-hub/sections/activity_monitor_interactive.tsx-113-115 (1)
113-115:⚠️ Potential issue | 🟡 MinorAvoid double scroll subscriptions for the same handler.
updateModalScrollStateis attached through bothaddEventListener("scroll", ...)andonScroll, which duplicates updates and re-renders. Keep only one path.🔧 Suggested fix
- <div - ref={modalScrollRef} - onScroll={updateModalScrollState} - className="flex h-full min-h-0 flex-col gap-2.5 overflow-y-auto pr-1 md:max-h-[min(70vh,42rem)]" - > + <div + ref={modalScrollRef} + className="flex h-full min-h-0 flex-col gap-2.5 overflow-y-auto pr-1 md:max-h-[min(70vh,42rem)]" + >Also applies to: 177-178
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/sections/activity_monitor_interactive.tsx around lines 113 - 115, The scroll handler updateModalScrollState is being wired twice (via scrollElement.addEventListener("scroll", updateModalScrollState, ...) and via the component's onScroll prop), causing duplicate updates and extra renders; pick one approach and remove the other—either stop calling scrollElement.addEventListener for scroll and rely on the component's onScroll, or remove the onScroll prop and keep the addEventListener—ensure to clean up the corresponding removal (removeEventListener) and any related useEffect logic that registers updateModalScrollState (also applied to the second occurrence around lines 177–178).front_end/src/app/(main)/labor-hub/page.tsx-308-309 (1)
308-309:⚠️ Potential issue | 🟡 MinorResolve the provisional activity link before release.
Line 308 explicitly flags the link as possibly wrong. Shipping this risks sending users to an incorrect destination from a prominent quote card.
I can help draft a quick validation checklist (question slug ↔ comment id ↔ post existence) and open a follow-up issue if you want.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/page.tsx around lines 308 - 309, The provisional hardcoded link assigned to the link variable (link="/questions/42220/change-in-us-bachelors-relative-to-2024-25/#comment-793578") must be resolved before release: replace this TODO by validating the question slug and comment id pair and ensuring the target post exists (e.g., add a resolver/validator function used when rendering the quote card that checks the question slug ↔ comment id and returns either the canonical comment URL or null), then use the validated URL for the link prop (or hide/remove the "original comment" link if validation fails); reference the link assignment in page.tsx and add/plug a validation helper used by the component that renders the quote card.front_end/src/app/(main)/labor-hub/components/activity_card.tsx-61-61 (1)
61-61:⚠️ Potential issue | 🟡 MinorLocalize the external-link accessibility label.
Line 61 uses a hardcoded English
aria-label; move it to i18n keys for consistency with localized UI.Based on learnings: Do not hardcode English strings in TSX components; prefer
useTranslations()and i18n strings for UI text across components.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/activity_card.tsx at line 61, The hardcoded aria-label="Open external link" in the ActivityCard component should be localized: import and call useTranslations() inside the ActivityCard (or the component containing that JSX), replace the literal aria-label with t('yourKey') (e.g., t('activity.externalLink')) and add the corresponding key/value to the i18n resource files for all supported locales; ensure the key name matches where you call t('activity.externalLink') and update any tests or story snapshots if they assert the raw string.front_end/src/app/(main)/labor-hub/components/question-cards/multi_question_table.tsx-1-223 (1)
1-223:⚠️ Potential issue | 🟡 MinorConsolidate duplicate file with inconsistent folder naming.
Two identical files exist:
question-cards/multi_question_table.tsx(hyphen) andquestion_cards/multi_question_table.tsx(underscore). Remove one and update all imports to use a single, consistent folder naming convention.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question-cards/multi_question_table.tsx around lines 1 - 223, There are duplicate files using inconsistent folder names (question-cards vs question_cards); pick one convention (e.g., question-cards), remove the duplicate file, and update all imports to reference the chosen path; search for uses of MultiQuestionTable, MultiQuestionTableContent, MultiQuestionTableSkeleton, reactNodeToText and FormattedValue to locate import sites and replace any import paths like "./question_cards/multi_question_table" or "question_cards/..." with the consolidated path "question-cards/..." (or vice versa if you choose underscores), then run the build to fix any remaining import errors.
🧹 Nitpick comments (33)
front_end/src/app/(main)/labor-hub/helpers/labor_hub_format.ts (1)
2-4: Consider i18n for user-facing text and handle zero edge case.The strings "shrink" and "grow" appear to be used in user-facing copy. If so, they should be localized. Additionally, when
value === 0, this returns"grow 0%"which may not be semantically ideal—consider returning a neutral phrasing like "unchanged" or "0%".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/helpers/labor_hub_format.ts around lines 2 - 4, Update the user-facing string handling in formatOccupationChange to use i18n/localization keys rather than hard-coded English and to treat zero as a neutral case: replace the literal "shrink"/"grow" outputs with translated strings (e.g., t('labor.shrink'), t('labor.grow')) and when value === 0 return a localized "unchanged" or "0%" message (e.g., t('labor.unchanged') or format with t('labor.percent', { value: 0 })) so all branches call the translation function and the zero edge case is handled semantically.front_end/src/app/(main)/labor-hub/helpers/fetch_jobs_data.ts (2)
65-73: Potential silent failure if aggregation method doesn't exist.
getSubQuestionValueaccessesquestion.aggregations[question.default_aggregation_method]which could returnundefinedif the method key doesn't exist in the aggregations object. The current code handles this gracefully by returningnull, but consider adding explicit validation or logging for debugging purposes in case of unexpected data shapes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/helpers/fetch_jobs_data.ts around lines 65 - 73, getSubQuestionValue silently returns null if question.aggregations[question.default_aggregation_method] is missing; update the function (getSubQuestionValue) to explicitly check whether question.aggregations and the key question.default_aggregation_method exist before accessing .latest, and log or throw a debug-level warning when the aggregation key is absent so unexpected data shapes are flagged; keep the existing null return for consumers but add a processLogger/debug log (or a console.warn) referencing question.default_aggregation_method and question.id (or another identifier) and then call scaleInternalLocation only when center is present.
58-60: Hardcoded baseline year assumption.The logic prepends
{ year: 2025, value: 0 }if the first data point isn't 2025. This embeds a business rule that may need updating over time. Consider extracting this as a constant or parameter.♻️ Extract baseline year constant
+const BASELINE_YEAR = 2025; +const BASELINE_VALUE = 0; + export const fetchOverallData = cache(async (): Promise<YearValue[]> => { // ... - if (points.length && points[0]?.year !== 2025) { - points.unshift({ year: 2025, value: 0 }); + if (points.length && points[0]?.year !== BASELINE_YEAR) { + points.unshift({ year: BASELINE_YEAR, value: BASELINE_VALUE }); } // ... });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/helpers/fetch_jobs_data.ts around lines 58 - 60, The code currently hardcodes a baseline year by inserting { year: 2025, value: 0 } when points[0]?.year !== 2025; extract that magic value into a single constant (e.g., BASELINE_YEAR or DEFAULT_BASELINE_YEAR) at the top of the module or accept it as a parameter to fetchJobsData, then replace the literal 2025 in the condition and the inserted object with that constant/parameter (update any related types and tests accordingly) so future changes only require updating the constant or passing a different baseline year.front_end/src/app/(main)/labor-hub/components/question_cards/chart_core/multi_line_chart_model.ts (1)
17-36: DuplicatewrapTextfunction.This is the same
wrapTextimplementation that exists inmulti_line_chart.tsx. As noted in that file's review, consider extracting to a shared utility.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question_cards/chart_core/multi_line_chart_model.ts around lines 17 - 36, The wrapText function in multi_line_chart_model.ts is a duplicate of the implementation in multi_line_chart.tsx; extract this logic into a shared utility (e.g., create a new exported function wrapText in a common utils file) and replace the local definition in both multi_line_chart_model.ts and multi_line_chart.tsx with imports from that utility; update references to the function name wrapText (and any callers in MultiLineChartModel or the MultiLineChart component) to use the single shared implementation and remove the duplicate definition.front_end/src/app/(main)/labor-hub/components/question_cards/placeholder.tsx (1)
27-29: Hardcoded English string should use i18n.The text "Question not available yet" should use
useTranslations()for localization consistency. Based on learnings: "Do not hardcode English strings in TSX components. Prefer using useTranslations() and i18n strings for UI text across components."♻️ Suggested refactor for i18n
+"use client"; + +import { useTranslations } from "next-intl"; + export function NoQuestionPlaceholder() { + const t = useTranslations(); return ( <div className="relative flex min-h-[200px] items-center justify-center overflow-hidden rounded"> {/* ... */} <div className="relative flex flex-col items-center gap-2"> <p className="text-sm font-medium text-gray-500 dark:text-gray-500-dark"> - Question not available yet + {t("questionNotAvailableYet")} </p> </div> </div> ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question_cards/placeholder.tsx around lines 27 - 29, Replace the hardcoded English string in the Placeholder component with a call to the i18n hook: import and call useTranslations() at the top of placeholder.tsx (e.g., const t = useTranslations()), add a new translation key (e.g., "laborHub.questionNotAvailable") to your locale files, and replace the literal "Question not available yet" in the <p> element with t('laborHub.questionNotAvailable') so the component uses localized text instead of a hardcoded string.front_end/src/app/(main)/labor-hub/components/question_cards/multi_line_chart.tsx (2)
255-257: Hardcoded aria-label should use i18n.The aria-label
More information about ${item.label}contains hardcoded English. Consider using a translation function with interpolation. Based on learnings about avoiding hardcoded English strings.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question_cards/multi_line_chart.tsx around lines 255 - 257, The hardcoded aria-label string in the span within MultiLineChart (the aria-label "More information about ${item.label}") must be replaced with an i18n call so it can be translated and still interpolate item.label; update the span's aria-label to use the project's translation helper (e.g., t or i18n.t) with a translation key like "more_information_about" and pass item.label as a variable, ensuring the translation file contains the corresponding key and interpolation placeholder; locate this in the MultiLineChart component where the span with className "inline-flex size-3.5 items-center justify-center rounded-full" is rendered and replace the literal string accordingly.
273-292: DuplicatedwrapTextutility function.This
wrapTextfunction is identical to the one inmulti_line_chart_model.ts(lines 17-36). Consider extracting it to a shared utility module to avoid duplication.♻️ Extract to shared utility
Create a shared utility file, e.g.,
chart_utils.ts:export const wrapText = (text: string, maxCharsPerLine: number): string[] => { if (text.length <= maxCharsPerLine) return [text]; const words = text.split(" "); const lines: string[] = []; let currentLine = ""; for (const word of words) { const testLine = currentLine ? `${currentLine} ${word}` : word; if (testLine.length <= maxCharsPerLine) { currentLine = testLine; } else { if (currentLine) lines.push(currentLine); currentLine = word; } } if (currentLine) lines.push(currentLine); return lines; };Then import it in both files.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question_cards/multi_line_chart.tsx around lines 273 - 292, The wrapText function is duplicated; extract it into a shared utility (e.g., chart_utils.ts) as a single exported function named wrapText, remove the duplicate implementations from multi_line_chart.tsx and multi_line_chart_model.ts, and update both files to import { wrapText } from the new module; ensure the exported function signature remains (text: string, maxCharsPerLine: number): string[] and that all existing call sites continue to work.front_end/src/app/(main)/labor-hub/components/question_cards/flippable_question_card.tsx (2)
57-58: Hardcoded aria-labels should use i18n.The aria-label strings "Show first view" and "Show second view" should be localized for accessibility in non-English locales. Based on learnings about avoiding hardcoded English strings.
Also applies to: 73-74
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question_cards/flippable_question_card.tsx around lines 57 - 58, The aria-label attributes in the FlippableQuestionCard component are hardcoded ("Show first view"/"Show second view"); replace them with localized messages using the app's i18n helper (e.g., use t(...) or intl.formatMessage) so both occurrences (the aria-label on the left/right toggle buttons around selected === "left" and the corresponding one for the right/second view) use translated strings; ensure you import and call the existing i18n function used elsewhere in the codebase and pass clear message keys like "flippableCard.showFirstView" and "flippableCard.showSecondView".
141-163: Both content regions are always rendered.Both
leftContentandrightContentare rendered simultaneously with one hidden via CSS. This is fine for simple content but may impact performance if children are expensive to render or have side effects. If this becomes a concern, consider conditionally rendering only the active side.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question_cards/flippable_question_card.tsx around lines 141 - 163, The two panels currently always render (leftContent and rightContent) and only hide via CSS; update the JSX in flippable_question_card.tsx to conditionally render only the active side based on currentSide (e.g., render leftContent when currentSide === "left" and rightContent when currentSide === "right") instead of rendering both divs; keep the existing wrapper and transition class logic but move it inside the conditional branches so expensive children or side-effectful components are not instantiated when not active.front_end/src/app/(main)/labor-hub/components/question-cards/multi_question_data.ts (1)
8-13: Naming inconsistency:questionIdvspostId.The
MultiQuestionRowConfig.questionIdfield is used to fetch posts (line 37:postIds = rows.map(row => row.questionId)), but semantically it represents a post ID based on usage withServerPostsApi.getPostsWithCP. Consider renaming topostIdfor clarity, or adding a comment explaining the naming rationale.Also applies to: 37-37
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question-cards/multi_question_data.ts around lines 8 - 13, The type field name questionId in MultiQuestionRowConfig is misleading because code treats it as a post ID (e.g., rows.map(row => row.questionId) feeding ServerPostsApi.getPostsWithCP); rename the field to postId in the MultiQuestionRowConfig type and update all usages (including variables like postIds, any rows.map calls, function params, and any components or helpers that consume this type) to use postId, or if renaming isn't possible, add a clear comment on MultiQuestionRowConfig.questionId explaining it represents a post ID and adjust local variable names to reflect postId for clarity.front_end/src/app/(main)/labor-hub/components/question_cards/question_card.tsx (6)
232-249: Remove unnecessary Fragment wrapper.The
<></>Fragment wrapping theDropdownMenuis redundant since there's only one child element.♻️ Suggested fix
return ( - <> - <DropdownMenu - items={moreMenuItems} - ... - > - ... - </DropdownMenu> - </> + <DropdownMenu + items={moreMenuItems} + ... + > + ... + </DropdownMenu> );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question_cards/question_card.tsx around lines 232 - 249, The returned JSX wraps a single DropdownMenu in an unnecessary Fragment; remove the surrounding <>...</> so the component directly returns the DropdownMenu element (containing Button, FontAwesomeIcon and faEllipsis) — update the return in question_card.tsx to return the DropdownMenu node itself without the fragment wrapper.
23-29: Hardcoded locale may break localization.The
formatCurrentDatefunction uses a hardcoded"en-US"locale. Consider using the user's locale from i18n context for consistent date formatting across the app.Based on learnings: "Do not hardcode English strings in TSX components... Prefer using useTranslations() and i18n strings for UI text across components."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question_cards/question_card.tsx around lines 23 - 29, The formatCurrentDate function currently hardcodes "en-US"; update it to use the app's runtime locale instead: obtain the user's locale from the i18n context (e.g., via useTranslations() / useLocale or the existing locale provider) and pass that locale into toLocaleDateString so date formatting follows the user's locale; modify the formatCurrentDate implementation to accept or read the locale (or convert it into a small wrapper that takes locale) and replace the hardcoded "en-US" with the retrieved locale while keeping the same options object.
161-226: Localize menu item labels.Menu labels like
"View Questions","Export PNG","Export CSV", and"Share on X"are hardcoded. Consider using i18n for these UI strings.Based on learnings: "Do not hardcode English strings in TSX components..."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question_cards/question_card.tsx around lines 161 - 226, The menu item labels in moreMenuItems are hardcoded English strings; update all label/name usages (the "View Questions"/"View Question", "Export PNG", "Export CSV", "Share on X") to use the app's i18n lookup (e.g., the translation function from your localization hook such as t or useTranslation) instead of literals so the MenuLinkWithIcon/MenuItemWithIcon props receive translated strings; ensure pluralization for hasMultiplePosts is handled via the translation keys or conditional translation, and replace occurrences passed to both name and label props (including where viewQuestionsHref, onExportPng, handleExportCsv, and shareOnTwitterLink are used) so all user-facing text is localized.
129-131: Localize toast messages.Toast strings like
"Preparing CSV export...","CSV download started.", and"Failed to export CSV..."are hardcoded in English. These should useuseTranslations()for i18n support.Based on learnings: "Do not hardcode English strings in TSX components... Prefer using useTranslations() and i18n strings for UI text across components."
Also applies to: 149-150, 153-154
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question_cards/question_card.tsx around lines 129 - 131, Replace hardcoded English toast messages in question_card.tsx by using the useTranslations() hook: import and call useTranslations() at the top of the QuestionCard component, then replace the strings passed to toast.loading (loadingToastId), toast.success, and toast.error with t('...') keys (e.g., t('export.preparingCsv'), t('export.started'), t('export.failed')) and add corresponding i18n entries; update the three places referenced (the toast.loading call that sets loadingToastId and the subsequent toast.success/toast.error calls) to use those translation keys.
337-343: Localize image export toast messages and type the error.The success/error messages are hardcoded English. Also, the error is typed as
any—preferunknownand narrow withinstanceof.♻️ Suggested improvement
- .catch((err: any) => { - console.error("Error exporting image:", err); - toast.error("Failed to export image"); + .catch((err: unknown) => { + console.error("Error exporting image:", err); + toast.error(t("failedToExportImage")); // Use i18n })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question_cards/question_card.tsx around lines 337 - 343, Replace hardcoded English toasts in the image export flow with localized strings (use your app's i18n/t function where the current toast("Image downloaded successfully", ...) and toast.error("Failed to export image") calls are located) and change the catch parameter from err: any to err: unknown; then narrow the error using instanceof Error before logging and extracting the message (e.g., console.error("Error exporting image:", errMessage)). Ensure you still call the success/error toast functions (toast or toast.error) but pass localized text (e.g., t('imageExport.success') / t('imageExport.failure')) and include the error.message when available.
419-423: Localize footer attribution text.The strings
"Data from"and"on"are hardcoded English. UseuseTranslations()for proper localization.Based on learnings: "Do not hardcode English strings in TSX components..."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question_cards/question_card.tsx around lines 419 - 423, The footer strings "Data from" and "on" are hardcoded; import and call useTranslations() inside the QuestionCard component to fetch localized strings (e.g., t('dataFrom') and t('on')) and replace the two literal span texts in the div that contains MetaculusTextLogo and formatCurrentDate(); also add the corresponding keys to the locale resource files so the translations exist. Ensure you keep formatCurrentDate() and MetaculusTextLogo as-is and only swap the literal spans to use the t(...) values.front_end/src/app/(main)/labor-hub/sections/jobs_monitor.tsx (3)
130-145: Address placeholder text before release.The insight cards contain
[TO BE CHANGED]markers indicating placeholder content. Ensure this is replaced with final copy before merging.Do you want me to open an issue to track replacing this placeholder text?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/sections/jobs_monitor.tsx around lines 130 - 145, The InsightCard instances positiveInsight and negativeInsight contain leftover placeholder markers "[TO BE CHANGED]"; replace those markers with the final copy (or pull the text from the approved content source / localization key) and ensure the strings around the year prop/variable remain correct; update both usages of InsightCard in jobs_monitor.tsx (the positiveInsight and negativeInsight definitions) to remove the placeholder tokens and confirm any changes are reflected where these variables are rendered.
85-86: Localize UI strings.The title
"Jobs Monitor"and subtitle are hardcoded English. UseuseTranslations()for i18n support.Based on learnings: "Do not hardcode English strings in TSX components..."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/sections/jobs_monitor.tsx around lines 85 - 86, The component is hardcoding English UI strings; replace the literal title="Jobs Monitor" and subtitle="AI is reshaping the job market, but not all fields are affected equally." with localized strings using useTranslations(): call useTranslations() (e.g., const t = useTranslations('JobsMonitor')) at the top of the JobsMonitor component (or function where those props are rendered) and pass title={t('title')} and subtitle={t('subtitle')}, adding corresponding keys ("title" and "subtitle") to the JobsMonitor i18n namespace/resource files.
104-121: Localize legend and label strings.Strings like
"(Percentage change in employment)","lower","higher","2025 Baseline","Expected growth", and"Expected decline"are hardcoded English.Based on learnings: "Do not hardcode English strings in TSX components..."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/sections/jobs_monitor.tsx around lines 104 - 121, The TSX block in jobs_monitor.tsx hardcodes UI strings (e.g., "(Percentage change in employment)", "lower", "higher", "2025 Baseline", and similar labels like "Expected growth"/"Expected decline"); replace these with calls to your app's i18n/localization API (e.g., useTranslation()/t or translate()) and corresponding translation keys, import the localization hook at the top of the JobsMonitor component, and update the JSX that renders BAR_SCALE labels and the baseline string to use t('jobs.legend.percentage_change'), t('jobs.legend.lower'), t('jobs.legend.higher'), t('jobs.legend.baseline_2025') (or your project's key naming) so all user-facing text is localized; also add the new keys to your locale files for supported languages.front_end/src/app/(main)/labor-hub/components/question-cards/multi_line_chart.tsx (1)
273-292:wrapTextfunction is duplicated.This
wrapTextfunction is identical to the one inmulti_line_chart_model.ts(lines 17-36). Consider extracting it to a shared utility to avoid duplication.♻️ Suggested approach
Extract
wrapTextto a shared utility file (e.g.,chart_utils.ts) and import it in both locations:// chart_utils.ts export const wrapText = (text: string, maxCharsPerLine: number): string[] => { // ... implementation };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question-cards/multi_line_chart.tsx around lines 273 - 292, The wrapText function in multi_line_chart.tsx is duplicated from multi_line_chart_model.ts; extract it into a shared utility (e.g., chart_utils.ts) as an exported function named wrapText(text: string, maxCharsPerLine: number): string[], replace the duplicate implementations by importing wrapText from that utility in both multi_line_chart.tsx and multi_line_chart_model.ts, and remove the local wrapText definitions so both files use the single shared implementation.front_end/src/app/(main)/labor-hub/sections/jobs_monitor_server.tsx (1)
12-17: Type assertions assume trusted data structure.The casts
row[0] as stringandrow[i + 1] as number | nullassume the API always returns correctly shaped data. IffetchJobsTableDatacould return malformed data, consider adding runtime validation to prevent server-side errors.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/sections/jobs_monitor_server.tsx around lines 12 - 17, The mapping that builds jobs from rows assumes trusted shapes (using row[0] as string and row[i + 1] as number | null) and should validate inputs from fetchJobsTableData at runtime; update the jobs creation (the rows -> jobs mapping in jobs_monitor_server.tsx) to check types for row[0] and each row[i + 1] against expected types (string and number|null), skip or filter malformed rows/values, and provide safe fallbacks or log warnings/errors instead of blind casting so JobRow and downstream code never receive invalid types.front_end/src/app/(main)/labor-hub/components/labor_hub_multi_line_chart.tsx (1)
9-21: Hide the controlled-highlight props from this wrapper's API.When hover context exists, this component always overwrites
highlightedX,onHighlightedXChange, andclearHighlightOnMouseLeave, so callers can currently pass props that silently do nothing. Narrowing the prop type withOmit<...>would make that contract explicit.♻️ Minimal type narrowing
+type LaborHubMultiLineChartProps = Omit< + MultiLineChartProps, + "highlightedX" | "onHighlightedXChange" | "clearHighlightOnMouseLeave" +>; + -export function LaborHubMultiLineChart(props: MultiLineChartProps) { +export function LaborHubMultiLineChart(props: LaborHubMultiLineChartProps) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/labor_hub_multi_line_chart.tsx around lines 9 - 21, The wrapper exposes props that it will always override (highlightedX, onHighlightedXChange, clearHighlightOnMouseLeave); change the component's prop type to explicitly omit those keys so callers can't pass them: update the LaborHubMultiLineChart signature to accept Omit<MultiLineChartProps, 'highlightedX' | 'onHighlightedXChange' | 'clearHighlightOnMouseLeave'> (and import MultiLineChartProps if needed), keep the use of useLaborHubChartHover and the overridden prop assignments inside the component as-is.front_end/src/app/(main)/labor-hub/sections/activity_monitor.tsx (1)
63-79: Consider using i18n for user-facing text.The section header "Activity Monitor" (Line 64) and the question title/subtitle (Lines 72-73) are hardcoded English strings. Per the codebase conventions, UI text should use
useTranslations()and i18n strings to support localization.If this is intentionally English-only for now, consider adding a TODO comment to track the i18n migration.
Based on learnings: "Do not hardcode English strings in TSX components... Prefer using useTranslations() and i18n strings for UI text across components."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/sections/activity_monitor.tsx around lines 63 - 79, The hardcoded UI strings ("Activity Monitor" and the QuestionLoader title/subtitle) should be replaced with i18n lookups using useTranslations(); update the component to import and call useTranslations() and pass translated strings into SectionHeader and QuestionLoader (referencing SectionHeader, ActivityMonitorInteractive, and QuestionLoader props) or, if intentionally English-only for now, add a TODO comment near those usages noting to migrate to i18n and include the i18n keys that will be needed (e.g., activityMonitor.title, question.41307.title, question.41307.subtitle) so reviewers can find and replace them later.front_end/src/app/(main)/labor-hub/components/labor_hub_navigation.tsx (1)
141-158: Minor: Consider i18n for the aria-label.The
aria-label="Subscribe for updates"is a hardcoded English string. For consistency with codebase conventions, consider using a translated string here as well.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/labor_hub_navigation.tsx around lines 141 - 158, The aria-label on the Button component is hardcoded; update it to use the project's i18n helper (e.g., t or useTranslation) instead of the literal "Subscribe for updates" so it appears in translated builds—replace aria-label="Subscribe for updates" in the Button (the element using ref={setReference}, aria-expanded={isNewsletterOpen}, and {...getReferenceProps()}) with the translation call (e.g., aria-label={t('subscribe_for_updates')}) ensuring the key exists in the locale files and importing/using the same i18n hook or function used elsewhere in this file.front_end/src/app/(main)/labor-hub/components/question_cards/question.tsx (2)
152-156: Consider extracting the note rendering to reduce duplication.The note rendering logic is duplicated in both the
isFlippableand non-flippable branches. Consider extracting it to a helper or rendering it once after the conditional.♻️ Proposed refactor to reduce duplication
+ const noteElement = note && ( + <div className="!mt-2 text-sm text-blue-700 dark:text-blue-700-dark"> + {note} + </div> + ); + if (isFlippable) { return ( <> <FlippableQuestionCard // ... props /> - {note && ( - <div className="!mt-2 text-sm text-blue-700 dark:text-blue-700-dark"> - {note} - </div> - )} + {noteElement} </> ); } return ( <> <QuestionCard> // ... content </QuestionCard> - {note && ( - <div className="!mt-2 text-sm text-blue-700 dark:text-blue-700-dark"> - {note} - </div> - )} + {noteElement} </> );Also applies to: 177-180
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question_cards/question.tsx around lines 152 - 156, The note JSX is duplicated in the isFlippable and non-flippable branches inside the Question component; extract the repeated block into a small helper or a single render spot to avoid duplication. Create a function or constant (e.g., renderNote or noteElement) that returns the JSX for {note} (the div with classes "!mt-2 text-sm text-blue-700 dark:text-blue-700-dark") and replace the duplicated instances in the isFlippable branch and the else branch with that helper, or alternatively render the helper once after the conditional render so both paths share the same noteElement.
102-116: Consider logging the error for debugging.The empty
catchblock silently swallows the error. While the fallback UI is appropriate, logging the error would help with debugging production issues.♻️ Proposed fix to add error logging
try { postData = await ServerPostsApi.getPost(questionId, true); - } catch { + } catch (error) { + console.error(`Failed to load question ${questionId}:`, error); return ( <QuestionCard🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question_cards/question.tsx around lines 102 - 116, The catch block that handles ServerPostsApi.getPost(questionId, true) silently swallows errors; update the catch to capture the error (e) and log it (e.g., console.error) along with context like questionId and maybe title before returning the fallback QuestionCard/NoQuestionPlaceholder; keep the fallback UI unchanged but ensure the error is logged to aid debugging of postData fetch failures.front_end/src/app/(main)/labor-hub/sections/sortable_research_table.tsx (1)
188-197: Consider using i18n for table headings.The heading "Metaculus Predicted Employment Change" (Line 189), "Occupation" (Line 196), and "AI Vulnerability Rating" (Line 233) are hardcoded English strings. Per codebase conventions, consider using translated strings for these UI labels.
Based on learnings: "Do not hardcode English strings in TSX components... Prefer using useTranslations() and i18n strings for UI text across components."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/sections/sortable_research_table.tsx around lines 188 - 197, Replace hardcoded UI strings in the SortableResearchTable TSX component with i18n keys: import and call useTranslations (e.g., const t = useTranslations('LaborHub' or appropriate namespace)) and replace the literal "Metaculus Predicted Employment Change", "Occupation", and "AI Vulnerability Rating" used inside the JSX (the div and TableCompactHeaderCell elements) with t('<key>') calls (choose descriptive keys like 'predictedEmploymentChange', 'occupation', 'aiVulnerabilityRating'); then add those keys and translations to the locale resource file for the component's namespace. Ensure the import of useTranslations and namespace choice is consistent with other components using i18n in this codebase.front_end/src/components/ui/scrollspy.tsx (2)
136-140: Minor redundancy:data-activeremoval is handled twice.
setActiveSection(lines 42-50) already removesdata-activefrom non-active anchors. This loop at lines 136-140 repeats the same operation. Not a bug, but could be removed for clarity.♻️ Consider removing the redundant loop
const activeAnchor = anchorElementsRef.current[activeIdx]; const sectionId = activeAnchor?.getAttribute(`data-${dataAttribute}-anchor`) || null; setActiveSection(sectionId); - // Remove data-active from all others - anchorElementsRef.current.forEach((item, idx) => { - if (idx !== activeIdx) { - item.removeAttribute("data-active"); - } - }); }, [anchorElementsRef, dataAttribute, offset, setActiveSection]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/components/ui/scrollspy.tsx` around lines 136 - 140, The loop that iterates anchorElementsRef.current and removes "data-active" for non-active items is redundant because setActiveSection already clears "data-active" from non-active anchors; remove the loop starting at the block referencing anchorElementsRef.current.forEach((item, idx) => { if (idx !== activeIdx) item.removeAttribute("data-active"); }) to avoid duplicate work and keep logic centralized in setActiveSection (ensure activeIdx usage and any remaining logic still rely on setActiveSection).
13-13: Unused prop:throttleTimeis declared but never used.The
throttleTimeprop is defined inScrollspyPropsbut is not destructured or used in the component. Either implement scroll throttling or remove this prop to avoid confusion.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/components/ui/scrollspy.tsx` at line 13, The declared prop throttleTime in ScrollspyProps is unused; remove it from the ScrollspyProps type and from any props destructuring inside the Scrollspy component (and from any default props/docs) so the prop no longer appears in the component API, and update any call sites that pass throttleTime to stop passing it; alternatively, if you prefer throttling behavior instead, wire throttleTime into the scroll handler inside the Scrollspy component (use a stable throttling helper or requestAnimationFrame wrapper), reference throttleTime when creating the onScroll listener in useEffect, and ensure cleanup still works — pick one approach and apply it consistently across ScrollspyProps, the Scrollspy component, and all usages.front_end/src/app/(main)/labor-hub/components/question_cards/multi_question_table.tsx (2)
88-88: Clarify naming:postIdscontains question IDs.The variable
postIdsis populated fromr.questionId, which may cause confusion. If question IDs and post IDs are interchangeable here, consider renaming for clarity; otherwise verifyMoreButtonhandles question IDs correctly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question_cards/multi_question_table.tsx at line 88, The variable postIds is misleading because it's derived from rows.map(r => r.questionId); rename postIds to questionIds (or questionIdsList) inside multi_question_table.tsx, update all usages where postIds is passed (notably the MoreButton prop) and adjust any prop/type names if necessary to reflect question IDs (ensure MoreButton’s prop and its type accept questionId(s) rather than postId if they differ); alternatively, if postId is the expected API shape, explicitly map questionId -> postId at the call site (e.g., rows.map(r => r.questionId) -> rows.map(r => ({ postId: r.questionId })) or rename the prop passed to MoreButton) so naming and types are consistent.
147-149: Hardcoded English string "Current" should use i18n.The fallback text "Current" on line 148 is hardcoded. This should be internationalized using the app's i18n setup to support localization. Based on learnings: "Do not hardcode English strings in TSX components."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question_cards/multi_question_table.tsx around lines 147 - 149, The hardcoded fallback "Current" in MultiQuestionTable should be replaced with a translated string: import and call the app i18n hook (e.g., useTranslations or t from your i18n lib) at the top of the component, then change {staticColumnHeader ?? "Current"} to {staticColumnHeader ?? t('laborHub.current')} (or the equivalent function call) and add the 'laborHub.current' key to your locale files; reference TableCompactHeaderCell and the staticColumnHeader fallback when making this change.front_end/src/app/(main)/labor-hub/components/table_compact.tsx (1)
79-80: Hardcoded "/hr" suffix should use i18n.The wage suffix "/hr" is hardcoded English. This should be internationalized to support localization (e.g., different languages may use different abbreviations or word order). Based on learnings: prefer i18n strings for UI text.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/table_compact.tsx around lines 79 - 80, The hardcoded wage suffix "/hr" in the render (the span next to ${value} inside table_compact.tsx) must be replaced with an i18n string; import and use your app's translation hook (e.g., useTranslation or t) in the component that renders this span (e.g., the TableCompact / wage cell render function) and replace the literal "/hr" with t('wage.perHour') or an agreed key (e.g., 'wage.perHourShort') so the suffix is localized; ensure the key is added to the translation files for all supported locales.front_end/src/app/(main)/labor-hub/components/question-cards/flippable_question_card.tsx (1)
57-58: Hardcoded English aria-labels should use i18n.The
aria-labelvalues "Show first view" and "Show second view" (lines 57, 73) are hardcoded. These accessibility strings should be internationalized. Based on learnings: prefer i18n strings for UI text.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question-cards/flippable_question_card.tsx around lines 57 - 58, The aria-label values in the FlippableQuestionCard component are hardcoded ("Show first view"/"Show second view"); replace them with i18n strings by importing and using the project's translation hook/function (e.g., useTranslation/useTranslations or t) and call it inside flippable_question_card.tsx to produce localized labels for the two toggle buttons (the elements using aria-label and aria-pressed and the counterpart button for the second view). Ensure you reference meaningful translation keys (e.g., "questionCard.showFirstView" and "questionCard.showSecondView") and pass the translated text to aria-label instead of the hardcoded English literals.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
front_end/src/app/(main)/labor-hub/components/question_cards/multi_question_line_chart.tsx (1)
224-238: Avoid repeated filtering/index lookup in historical tick thinning.Lines 230-232 rebuild and scan historical columns for every tick. Precomputing an index map once will reduce overhead and simplify the predicate.
♻️ Refactor sketch
+ const historicalIndexByLabel = new Map( + dataset.columns + .filter((column) => historicalLabelSet.has(column)) + .map((column, idx) => [column, idx] as const) + ); + const visibleXTickValues = historicalTickEvery && historicalTickEvery > 1 ? xTickValues.filter((xTickValue) => { const label = xTickLabelsByValue[String(xTickValue)]; if (!label || !historicalLabelSet.has(label)) return true; - - const historicalIndex = dataset.columns - .filter((column) => historicalLabelSet.has(column)) - .indexOf(label); + const historicalIndex = historicalIndexByLabel.get(label); return ( - historicalIndex === -1 || + historicalIndex == null || historicalIndex % historicalTickEvery === 0 ); }) : xTickValues;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question_cards/multi_question_line_chart.tsx around lines 224 - 238, The filter for visibleXTickValues repeatedly rebuilds and searches dataset.columns for each tick; fix by precomputing a map of historical column label -> index once (e.g. before computing visibleXTickValues) using dataset.columns.filter(...) and building an object or Map, then in the predicate for visibleXTickValues use that lookup (referencing historicalLabelSet, xTickLabelsByValue, and historicalTickEvery) to get historicalIndex instead of calling .filter(...).indexOf(label) each time; ensure the predicate still returns true when label is missing or historicalIndex % historicalTickEvery === 0.front_end/src/app/(main)/labor-hub/components/labor_hub_navigation.tsx (1)
26-31: NarrownewsletterListKeyto the supported list keys.
newsletterListKey?: stringlets typos compile. Infront_end/src/app/(main)/actions.ts:17-24, an unknown key becomesundefined, andfront_end/src/services/api/misc/misc.server.ts:105-122then falls back to the default Mailjet list. That silently subscribes Labor Hub users to the wrong audience instead of failing fast. Please type this as a sharedNewsletterListKey/keyof typeof NEWSLETTER_LIST_IDS.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/labor_hub_navigation.tsx around lines 26 - 31, The prop newsletterListKey on the LaborHubNavigation component is too permissive (string) causing silent fallback to the default Mailjet list; narrow its type to the shared union type (e.g., NewsletterListKey or keyof typeof NEWSLETTER_LIST_IDS) and import that type where the component is declared, then update any callers to pass only that typed key; ensure the shared type is the same one used by the logic in actions.ts and misc.server.ts so unknown keys become a compile error rather than runtime undefined/fallback.front_end/src/app/(main)/labor-hub/components/question_cards/multi_line_chart.tsx (1)
532-534: Keep badge formatting and axis formatting decoupled.Line 534 makes
formatYValuethe implicit fallback forformatYTick. That couples two separate props, and the badge formatter also feedscomputeMultiLineChartModel(), so a caller can't change badge text without also affecting tick text and left-padding calculation.Proposed fix
const formatResolvedYValue = formatYValueProp ?? defaultFormatYValue; - const formatResolvedYTick = - formatYTick ?? formatYValueProp ?? defaultFormatYTick; + const formatResolvedYTick = formatYTick ?? defaultFormatYTick;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/labor-hub/components/question_cards/multi_line_chart.tsx around lines 532 - 534, formatResolvedYTick currently falls back to formatYValueProp, coupling badge/label formatting with axis tick formatting; change formatResolvedYTick so it only uses formatYTick or defaultFormatYTick (i.e., remove formatYValueProp from the tick fallback) and ensure computeMultiLineChartModel continues to receive the badge formatter (formatYValueProp) separately so callers can change badge text without affecting axis ticks or left-padding calculations.
🤖 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)/labor-hub/components/question_cards/multi_line_chart.tsx:
- Around line 517-520: The code builds highlightedX from
highlightedXProp/uncontrolledHighlightedX but usePrintOverride() only suppresses
hover badges, so hover state still affects rendering; create a
renderedHighlightedX that returns null when usePrintOverride() indicates print
mode (or otherwise forces no hover) and replace uses of highlightedX in the
x-tick formatter, x-axis tick emphasis logic, the vertical guide line render,
and DataPointCircle sizing/highlighting with renderedHighlightedX (look for
usages around the x-tick formatter, axis emphasis code, guide line drawing, and
DataPointCircle component calls such as those in the blocks mentioned) so that
in print mode those hover-only visuals are disabled.
- Around line 632-656: The pointer-move handler handleChartMouseMove currently
clears the shared highlight whenever the pointer enters the left/right gutter
(it calls setHighlightedX(null) in the offsetX-out-of-plot branch), which breaks
the shared hover year; update that branch to only clear the highlight when a
prop clearHighlightOnMouseLeave is true (otherwise just return without calling
setHighlightedX), and add clearHighlightOnMouseLeave to the hook dependency
array so the behavior follows the prop passed from labor_hub_multi_line_chart.
In
`@front_end/src/app/`(main)/labor-hub/components/question_cards/multi_question_line_chart.tsx:
- Around line 217-223: historicalLabelSet is currently derived from rows while
the chart builds historical points from dataset.rows, which can cause
divider/tick drift when resolution changes; change the source-of-truth so the
set is built from dataset.rows (i.e., use Object.keys(row.historicalValues ??
{}) for each row in dataset.rows instead of rows) and pass that resulting set
into getHistoricalForecastDividerX along with dataset.columns so divider/tick
logic and the rendered historical points use the same data source (update the
usage of historicalLabelSet, rows, dataset.rows, and
getHistoricalForecastDividerX accordingly).
In `@front_end/src/app/`(main)/labor-hub/components/scrollspy_button_group.tsx:
- Around line 22-29: The buttons are only visually indicating the active section
via the data-active attribute; update the button rendering in
scrollspy_button_group.tsx to mirror that state into an ARIA attribute so
assistive tech can detect it: when the button with
data-scrollspy-anchor={item.id} is marked active (data-active="true"), set
aria-current="location" (and remove/set it when not active) — implement this by
adding a conditional aria-current prop tied to the same active condition used
for the data-active class toggling in the component.
---
Nitpick comments:
In `@front_end/src/app/`(main)/labor-hub/components/labor_hub_navigation.tsx:
- Around line 26-31: The prop newsletterListKey on the LaborHubNavigation
component is too permissive (string) causing silent fallback to the default
Mailjet list; narrow its type to the shared union type (e.g., NewsletterListKey
or keyof typeof NEWSLETTER_LIST_IDS) and import that type where the component is
declared, then update any callers to pass only that typed key; ensure the shared
type is the same one used by the logic in actions.ts and misc.server.ts so
unknown keys become a compile error rather than runtime undefined/fallback.
In
`@front_end/src/app/`(main)/labor-hub/components/question_cards/multi_line_chart.tsx:
- Around line 532-534: formatResolvedYTick currently falls back to
formatYValueProp, coupling badge/label formatting with axis tick formatting;
change formatResolvedYTick so it only uses formatYTick or defaultFormatYTick
(i.e., remove formatYValueProp from the tick fallback) and ensure
computeMultiLineChartModel continues to receive the badge formatter
(formatYValueProp) separately so callers can change badge text without affecting
axis ticks or left-padding calculations.
In
`@front_end/src/app/`(main)/labor-hub/components/question_cards/multi_question_line_chart.tsx:
- Around line 224-238: The filter for visibleXTickValues repeatedly rebuilds and
searches dataset.columns for each tick; fix by precomputing a map of historical
column label -> index once (e.g. before computing visibleXTickValues) using
dataset.columns.filter(...) and building an object or Map, then in the predicate
for visibleXTickValues use that lookup (referencing historicalLabelSet,
xTickLabelsByValue, and historicalTickEvery) to get historicalIndex instead of
calling .filter(...).indexOf(label) each time; ensure the predicate still
returns true when label is missing or historicalIndex % historicalTickEvery ===
0.
🪄 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: 2433e559-12fe-401c-bd9a-763107cc52ed
📒 Files selected for processing (10)
front_end/src/app/(main)/labor-hub/components/labor_hub_navigation.tsxfront_end/src/app/(main)/labor-hub/components/mobile_carousel.tsxfront_end/src/app/(main)/labor-hub/components/question_cards/basic_question.tsxfront_end/src/app/(main)/labor-hub/components/question_cards/multi_line_chart.tsxfront_end/src/app/(main)/labor-hub/components/question_cards/multi_question_line_chart.tsxfront_end/src/app/(main)/labor-hub/components/question_cards/question.tsxfront_end/src/app/(main)/labor-hub/components/scrollspy_button_group.tsxfront_end/src/app/(main)/labor-hub/page.tsxfront_end/src/app/(main)/labor-hub/sections/activity_monitor.tsxfront_end/src/app/(main)/questions/[id]/components/question_view/consumer_question_view/prediction/single_question_prediction/continuous_question_prediction.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- front_end/src/app/(main)/labor-hub/components/mobile_carousel.tsx
- front_end/src/app/(main)/labor-hub/components/question_cards/question.tsx
- front_end/src/app/(main)/labor-hub/page.tsx
- front_end/src/app/(main)/labor-hub/sections/activity_monitor.tsx
- front_end/src/app/(main)/labor-hub/components/question_cards/basic_question.tsx
| const highlightedX = | ||
| highlightedXProp === undefined | ||
| ? uncontrolledHighlightedX | ||
| : highlightedXProp; |
There was a problem hiding this comment.
Print mode still renders hover artifacts.
usePrintOverride() only suppresses hover badges right now. A non-null highlightedX still changes tick visibility, bolds the active x tick, enlarges points, and draws the guide line, so printing after a hover can bake a transient active year into the chart.
Proposed fix
const highlightedX =
highlightedXProp === undefined
? uncontrolledHighlightedX
: highlightedXProp;
+ const renderedHighlightedX = isPrintMode ? null : highlightedX;Then use renderedHighlightedX instead of highlightedX anywhere the UI renders hover-only state: the x-tick formatter, x-axis tick emphasis, the vertical guide line, and DataPointCircle.
Also applies to: 664-699, 748-755, 816-831, 919-920
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@front_end/src/app/`(main)/labor-hub/components/question_cards/multi_line_chart.tsx
around lines 517 - 520, The code builds highlightedX from
highlightedXProp/uncontrolledHighlightedX but usePrintOverride() only suppresses
hover badges, so hover state still affects rendering; create a
renderedHighlightedX that returns null when usePrintOverride() indicates print
mode (or otherwise forces no hover) and replace uses of highlightedX in the
x-tick formatter, x-axis tick emphasis logic, the vertical guide line render,
and DataPointCircle sizing/highlighting with renderedHighlightedX (look for
usages around the x-tick formatter, axis emphasis code, guide line drawing, and
DataPointCircle component calls such as those in the blocks mentioned) so that
in print mode those hover-only visuals are disabled.
| if (typeof window === "undefined") { | ||
| return ""; | ||
| } | ||
| const url = `${window.location.origin}${pathname}${searchParams.toString() ? `?${searchParams.toString()}` : ""}${window.location.hash}`; | ||
| return `https://twitter.com/intent/tweet?text=${encodeURIComponent( | ||
| message | ||
| )}&url=${encodeURIComponent(url)}`; | ||
| }, [message, pathname, searchParams]); | ||
| const url = `${PUBLIC_APP_URL}${pathname}${searchParams.toString() ? `?${searchParams.toString()}` : ""}`; | ||
| return `https://x.com/intent/tweet?text=${encodeURIComponent(message)}&url=${encodeURIComponent(url)}`; | ||
| }, [PUBLIC_APP_URL, message, pathname, searchParams]); |
There was a problem hiding this comment.
Seems like Twitter/Facebook share hooks dropped ${window.location.hash} and switched to PUBLIC_APP_URL. I guess it causes site-wide regression: every page that shares a hash-anchored URL now loses the fragment.
| // TODO: Add link to original comment this can be wrong link for demo | ||
| link="/questions/42220/change-in-us-bachelors-relative-to-2024-25/#comment-793578" |
| </span>{" "} | ||
| sectors (employing 10% and 2%, respectively). | ||
| </ContentParagraph> | ||
| <ActivityCard username="John Doe" subtitle="Pro Forecaster"> |
| <InsightCard context="positive"> | ||
| [TO BE CHANGED] By {year}, roles in{" "} | ||
| <strong>construction</strong> and <strong>healthcare</strong>{" "} |
| typeof window !== "undefined" | ||
| ? window.location.hostname + window.location.pathname | ||
| : ""; |
There was a problem hiding this comment.
typeof window ternary in render body of a "use client" component causes hydration mismatch (server "", client URL).
| } from "@fortawesome/free-solid-svg-icons"; | ||
| import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; | ||
| import { saveAs } from "file-saver"; | ||
| import { toPng } from "html-to-image"; |
There was a problem hiding this comment.
used only in one callback, can be dynamically imported
Summary by CodeRabbit
New Features
Improvements