feat: improve i18n (lunaria) status page#2064
feat: improve i18n (lunaria) status page#2064alex-key wants to merge 3 commits intonpmx-dev:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
📝 WalkthroughWalkthroughThis pull request refactors the Lunaria components to use an I18nStatus type instead of LunariaStatus, removing dependencies on the LunariaInstance parameter from public component signatures. The changes include introducing new helper components (MissingKeysList, ContentDetailsLinks, ProgressBar, Link, TitleParagraph), updating the rendering logic to display locale-specific progress metrics, and deferring HTML dashboard generation to after jsonStatus construction. Stylesheet updates introduce new colour tokens and comprehensive progress-related styling to support the updated UI structure. Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
lunaria/components.ts (1)
260-261: Avoid!afterfind().If
localizationsdoes not containlang, this throws before any fallback status can be rendered. A small guard keeps this exported helper safe if the per-file view is brought back.As per coding guidelines, "Ensure you write strictly type-safe code, for example by ensuring you always check when accessing an array value by index".Safer fallback
- const localization = localizations.find(l => l.lang === lang)! + const localization = localizations.find(l => l.lang === lang) + if (!localization) { + return html`<td>${EmojiFileLink(null, 'missing')}</td>` + }
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8dd22553-934d-4d27-9362-b2a69b3d653e
📒 Files selected for processing (3)
lunaria/components.tslunaria/lunaria.tslunaria/styles.ts
| </summary> | ||
| ${outdatedFiles.length > 0 ? OutdatedFiles(outdatedFiles, lang, lunaria) : ''} | ||
| <br /> | ||
| ${ContentDetailsLinks({ text: `i18n/locales/${lang}.json`, url: githubEditUrl }, githubHistoryUrl)} |
There was a problem hiding this comment.
Render the resolved locale filename here.
The edit/history URLs are already built from the resolved path, but this label is reconstructed from lang. For locales whose primary edit target is a base file, the page will still say i18n/locales/ar-EG.json or i18n/locales/es-ES.json even though the link opens ar.json or es.json. Please carry the resolved file path through I18nStatus and render that instead.
🧭 Context
i18n status page was designed to track multiple files. Since we are using a single file for each location - it could be improved
📚 Description
The following changes were done:
jsonStatusobject instead of lunaria-generated status objectBefore
After