fix: various improvements for charts#1675
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
📝 WalkthroughWalkthroughThis pull request updates localisation strings in multiple files (i18n/locales/en.json, i18n/locales/fr-FR.json, lunaria/files/en-GB.json, lunaria/files/en-US.json, lunaria/files/fr-FR.json) by inserting a period after the 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)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
i18n/locales/en.jsoni18n/locales/fr-FR.jsonlunaria/files/en-GB.jsonlunaria/files/en-US.jsonlunaria/files/fr-FR.jsontest/unit/app/utils/charts.spec.ts
Pull request was converted to draft
There was a problem hiding this comment.
🧹 Nitpick comments (2)
test/unit/app/utils/charts.spec.ts (2)
793-804: Add boundary tests for emptylines/barsdatasets.The suite now covers
dataset: null, but not structurally empty datasets. Adding those cases will lock behaviour for index-based paths and prevent silent regressions.Proposed test additions
describe('createAltTextForTrendLineChart', () => { + it('handles dataset with empty lines without throwing', () => { + const { translate } = createTranslateMock() + const trendLineConfig = createTrendLineConfig({ $t: translate }) + + expect(() => + createAltTextForTrendLineChart({ + dataset: { lines: [] }, + config: trendLineConfig, + } as AltCopyArgs<TrendLineDataset, TrendLineConfig>), + ).not.toThrow() + }) + it('returns empty string when dataset is null', () => { const translateMock = createTranslateMock() const trendLineConfig = createTrendLineConfig({ $t: translateMock.translate }) @@ describe('createAltTextForVersionsBarChart', () => { + it('handles dataset with empty bars without throwing', () => { + const { translate } = createTranslateMock() + const config = createVersionsBarConfigForTests({ $t: translate as any }) + + expect(() => + createAltTextForVersionsBarChart({ + dataset: { bars: [] }, + config, + } as AltCopyArgs<VersionsBarDataset, VersionsBarConfig>), + ).not.toThrow() + }) + it('returns empty string when dataset is null (does not translate)', () => { const { translate, calls } = createTranslateMock()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".
Also applies to: 1055-1066
35-130: Use narrower types for fixture builders to maintain type safety.The fixture helpers (
createTrendLineConfig,createDatasetWithSingleLine,createDatasetWithTwoLines,createConfig,createDataset,createVersionsBarConfigForTests,createVersionsBarDatasetForTests) use broadas anyandas unknown ascasts. While the tests currently work, these casts can mask contract changes ifTrendLineConfig,TrendLineDataset,VersionsBarConfig, orVersionsBarDatasetare updated. Consider using more specific types—for example, casting individual items toPartial<VueUiXyDatasetLineItem>rather thanany, or leveraging TypeScript's type inference more directly. This aligns with the requirement to write strictly type-safe code.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/unit/app/utils/charts.spec.ts (1)
35-52: Consider consolidating similar factory functions.There are two config factory functions (
createTrendLineConfigandcreateConfig) with overlapping purposes. Additionally,createTrendLineConfigcreates an internaltranslateMockon line 36 that gets discarded when callers pass their own$toverride.This is a minor observation and doesn't affect test correctness, but consolidating these factories could improve maintainability.
Also applies to: 77-91
2 participants
Uh oh!
There was an error while loading. Please reload this page.