feat: updating react-dora-charts for compatibility with service-level processing #74
Conversation
* fix: initial change for team to service * fix: other team to service changes * docs: prettier readme * docs: update snapshots * docs: update snapshots, additional * fix: update snapshots for linux
BREAKING CHANGE: The component-based API has been replaced with a service-based approach.
## [2.0.0](v1.1.10...v2.0.0) (2025-04-01) ### ⚠ BREAKING CHANGES * The component-based API has been replaced with a service-based approach. ### Features * API to use service instead of component ([#73](#73)) ([638e49a](638e49a))
…sing && improved data bar tool tip
… && improved data bar tool tip
![windsurf-bot[bot]](https://avatars.githubusercontent.com/in/1066231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Other comments (5)
- src/RecoverTimeGraph.tsx (35-42) The copyToClipboard function logs success to the console but doesn't provide any visual feedback to the user. Consider adding a temporary visual indicator (like a small toast notification) to inform users when the SHA has been successfully copied.
- src/RecoverTimeGraph.tsx (170-172) The tooltip always converts repository recovery times to minutes (`timeValue = repo.time * 60`), even for longer recovery times that might be better displayed in hours or days. Consider using the same unit conversion logic that's applied to the service-level data to maintain consistency.
- src/RecoverTimeGraph.tsx (420-420) Setting the tooltip style with `position: 'fixed'` may cause positioning issues when scrolling or in certain layouts. Consider using `position: 'absolute'` with appropriate z-index instead, or leveraging the positioning options provided by the tooltip library.
- src/chart.module.css (41-41) Using `position: fixed` for tooltips might cause issues with scrolling, as the tooltip will remain in the same position relative to the viewport rather than staying with its trigger element. Consider whether `position: absolute` would be more appropriate for chart tooltips that should move with their parent elements during scrolling.
- src/chart.module.css (36-36) The z-index value of 9999 is extremely high. Consider using a more moderate value that's just high enough to ensure the tooltip appears above other elements. Excessively high z-index values can make it difficult to manage stacking contexts in larger applications.
| className={styles.toolTipLink} | ||
| href={deployment.url} | ||
| target="_blank" | ||
| > |
There was a problem hiding this comment.
Missing security attributes in the success deployments section. When using target="_blank", it's recommended to also include rel="noopener noreferrer" to prevent potential security vulnerabilities. This is correctly implemented in the failure deployments section (lines 162-163) but missing in the success deployments section.
| className={styles.toolTipLink} | |
| href={deployment.url} | |
| target="_blank" | |
| > | |
| className={styles.toolTipLink} | |
| href={deployment.url} | |
| target="_blank" | |
| rel="noopener noreferrer" |
| <div style={{ display: 'flex', alignItems: 'center', width: '100%' }}> | ||
| {/* Repository column - fixed width with ellipsis */} | ||
| <div style={{ minWidth: '120px', maxWidth: '200px', overflow: 'hidden', textOverflow: 'ellipsis', paddingRight: '12px', flexGrow: 0 }}> | ||
| <span title={deployment.repo}>{deployment.repo}</span> |
There was a problem hiding this comment.
There's an inconsistency in the flexGrow property between success and failure deployment repository columns. For success deployments it's set to 1 (line 96), but for failure deployments it's set to 0 (line 153). This will cause different layout behaviors between the two sections. Consider using the same value for consistency.
src/ChangeFailureRateGraph.tsx
Outdated
| <span | ||
| onClick={() => copyToClipboard(fullSha)} | ||
| style={{ | ||
| cursor: 'pointer', | ||
| fontSize: '1.2em', | ||
| color: '#333', | ||
| fontWeight: 'bold' | ||
| }} | ||
| title="Copy full SHA" | ||
| > | ||
| ⎘ | ||
| </span> |
There was a problem hiding this comment.
The SHA copy button implementation is inconsistent between success and failure deployments. In the success section (lines 112-128), it's in a separate div, but in the failure section (lines 168-179), it's inside the SHA column div. This creates different layouts and could make future maintenance more difficult.
jarrettgaither
force-pushed
the
main
branch
2 times, most recently
from
0b18ab3 to
0162a28
Compare
Updating the charts to process data based on the service name instead of the repository, as that is what V2 of the DORA API does. Also changed the tooltips that pop-up when you hover over data in the graph to hold relevant information and links.