Added new warning icon for missing data#181
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new data gap detection feature to the Power BI Multi-KPI visual, allowing users to detect and display warnings about missing days in time-series data. The feature parallels the existing stale data functionality.
- Adds data gap detection with configurable warning messages
- Displays gap indicators in the subtitle section with tooltips
- Provides per-series gap information when multiple series have different gaps
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/dataGapDetector.ts | New utility class that detects gaps in time-series data and formats warning messages |
| src/settings/descriptors/dataGapDescriptor.ts | New settings descriptor for data gap configuration including colors and message template |
| src/converter/data/dataRepresentation.ts | Adds IDataGapInfo interface to store gap detection results |
| src/converter/data/dataConverter.ts | Integrates gap detection into data processing pipeline |
| src/visualComponent/subtitleWarningComponent.ts | Adds rendering logic for data gap warnings with tooltips |
| src/visualComponent/rootComponent.ts | Passes data gap settings to subtitle component |
| src/settings/settings.ts | Registers data gap descriptor in settings model |
| capabilities.json | Defines data gap properties in Power BI capabilities |
| styles/styles.less | Adds CSS styling for data gap icon |
| stringResources/en-US/resources.resjson | Adds localized strings for data gap feature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| const days = this.daysBetween(prev.x, curr.x); | ||
|
|
||
| if (!isPrevValid || !isCurrValid || days > 1) { |
There was a problem hiding this comment.
This if logic looks non-clear. Add comment in the beginning, explaining in short, what we check and what we do.
There was a problem hiding this comment.
Added a clear comment above the if-logic explaining what we check: invalid values or missing days between two dates.
There was a problem hiding this comment.
does "more than 1 day is missing between the dates" is required ? It means at least 2 days, or you want to tell "1 day or more are missing ..."?
There was a problem hiding this comment.
The intention here is exactly “more than 1 day”, meaning 2 or more missing days between the dates.
A single missing day is treated as normal for time-series data (for example weekends or natural daily breaks), so I don’t mark it as a gap to avoid false-positive warnings.
Technically it’s possible to treat even a 1-day break as a gap, but I think that would generate unnecessary alerts.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 14 out of 15 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/utils/dataGapDetector.ts
Outdated
| // - more than 1 day is missing between the dates | ||
| if (!isPrevValid || !isCurrValid || days > 1) { | ||
| if (!currentGapStart) currentGapStart = prev.x; | ||
| gapDays += days > 1 ? days - 1 : 1; // If days are missing, count them; otherwise count this invalid point |
There was a problem hiding this comment.
The comment suggests that invalid points are counted as '1' missing day, but this logic could be misleading. When days <= 1 but a point is invalid (NaN, null, or Infinity), incrementing by 1 doesn't represent 'missing days' between dates—it represents an invalid data point. Consider clarifying that this counts invalid points rather than missing days, or revising the logic to distinguish between date gaps and invalid values.
| gapDays += days > 1 ? days - 1 : 1; // If days are missing, count them; otherwise count this invalid point | |
| gapDays += days > 1 ? days - 1 : 1; // If days are missing, count them; otherwise count this invalid data point (not a missing day) |
There was a problem hiding this comment.
I clarified the comment to clearly distinguish between: date gaps (days > 1 → count missing calendar days) invalid data points (NaN/null/Infinity → counted as 1 data issue, not a missing day) ․ The logic stays the same; only the explanation is now clearer.
|
|
||
| const days = this.daysBetween(prev.x, curr.x); | ||
|
|
||
| if (!isPrevValid || !isCurrValid || days > 1) { |
There was a problem hiding this comment.
does "more than 1 day is missing between the dates" is required ? It means at least 2 days, or you want to tell "1 day or more are missing ..."?
2 participants
No description provided.