Frontend code quality cleanup: LandingPage duplication, i18n any, CSS dup, double loading fallback#1238
Open
samuelisi wants to merge 4 commits into
Open
Conversation
…gPage The three feature cards, four how-it-works steps, and three footer sections were each near-identical hand-copied JSX blocks. Extract shared components and drive them from small local data arrays via .map() to remove the duplication and shrink LandingPage.tsx. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The i18n.t() lookup used `let value: any` to walk the translation tree, bypassing strict type checking for the function that resolves all UI copy. Type it as Translations | string and rely on the existing typeof narrowing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
.visually-hidden was defined identically in accessibility.css (globally imported) and again verbatim in LoadingSpinner.css and Skeleton.css. Remove the duplicates and rely on the global rule. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…page next/dynamic's ssr: true already manages its own loading boundary, so wrapping it in an outer <Suspense> rendered two different fallbacks (with mismatched aria-labels) for the same loading state. Drop the outer Suspense and keep a single consistent fallback. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
@samuelisi Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #1176
closes #1177
closes #1178
closes #1179
Summary
FeatureCard,Step,FooterColumncomponents (newsrc/components/landing/) and drive them from local data arrays inLandingPage.tsx, removing the hand-copied JSX blocks and shrinking the file (355 → 314 lines).i18n.tstranslation lookup asTranslations | stringinstead ofany, narrowing with the existingtypeofchecks..visually-hiddenrule fromLoadingSpinner.cssandSkeleton.css; the single definition in the globally-importedaccessibility.cssnow applies everywhere.<Suspense>around thessr: truenext/dynamicimport inapp/page.tsx, which rendered two different loading fallbacks with mismatchedaria-labels for the same loading state. One fallback remains, with a consistent label.Test plan
LandingPage.dataDriven.test.tsxasserting feature cards, steps, and footer columns render from the data arrays with translated copyi18n.test.tscases covering the non-string terminal-value branch (proves theanyremoval didn't change behavior)accessibility.css.test.tsasserting exactly one.visually-hiddendefinition exists across the stylesheet treepage.test.tsxto renderHomeand assert exactly one loading indicator (consistentaria-label="Loading page") appears during the dynamic import, and it's replaced onceLandingPageresolvescd frontend && npm test— 210/210 tests passingcd frontend && npm run build— builds cleanly with zero errorsKnown gap (pre-existing, not introduced by this PR)
npm run lintis currently broken onmain: Next.js 16 removed thenext lintsubcommand and there's no standalone ESLint config/dependency in the repo. This predates these changes — flagging since the tickets ask for a clean lint pass.🤖 Generated with Claude Code