Conversation
c0d71b1 to
203ae7d
Compare
butsyk
approved these changes
Dec 8, 2025
There was a problem hiding this comment.
Pull request overview
This pull request adds data update functionality to all graph renderers, enabling dynamic re-rendering while preserving persistent properties like selected time ranges. It also enhances time axis formatting by introducing a new "quarters" scale and refining existing scales (days, weeks, months) with improved tick and label formatting.
Key changes:
- Implements
updateData()andclearGraph()methods across all renderer classes to support dynamic data updates - Adds quarterly time scale support and improves time axis formatting logic in
UIControlsRenderer - Introduces
getEventNamesToRemove()pattern for proper event listener cleanup in renderer subclasses
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
src/graphs/Renderer.js |
Adds base updateData() and clearGraph() methods to support data updates across all renderers |
src/graphs/UIControlsRenderer.js |
Implements quarterly time scale, refines time axis formatting for days/weeks/months/quarters, and updates reporting range thresholds |
src/graphs/cfd/CFDRenderer.js |
Implements updateData() with time range preservation logic, updates clearGraph() for event cleanup, adds debug console.logs, and changes brush axis to use quarters |
src/graphs/scatterplot/ScatterplotRenderer.js |
Updates clearGraph() to use getEventNamesToRemove() pattern and handle optional brush element |
src/graphs/control-chart/ControlRenderer.js |
Adds getEventNamesToRemove(), clearGraph(), and updateData() implementations |
src/graphs/moving-range/MovingRangeRenderer.js |
Adds getEventNamesToRemove(), clearGraph(), and updateData() implementations |
src/graphs/histogram/HistogramRenderer.js |
Adds updateData() method and updates clearGraph() to call getEventNamesToRemove() |
src/graphs/work-item-age/WorkItemAgeRenderer.js |
Implements updateData() with data filtering and clearGraph() with event cleanup |
src/graphs/pbc/PBCRenderer.js |
Adds updateData() to coordinate updates between control and moving range renderers, adds getEventNamesToRemove(), and updates clearGraph() signature |
Comments suppressed due to low confidence (1)
src/graphs/histogram/HistogramRenderer.js:77
- The
clearGraphmethod inHistogramRendererdiffers from other renderers by re-drawing the SVG and axes after clearing. This behavior is inconsistent with the pattern used inWorkItemAgeRenderer,MovingRangeRenderer,ControlRenderer, and the baseRendererclass, whereclearGraphonly removes elements. The re-drawing logic should be inupdateDataorrenderGraphinstead. Consider removing lines 76-77 to maintain consistency.
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
8fede10 to
4922fe0
Compare
4922fe0 to
a61dfc6
Compare
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
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.
No description provided.