Skip to content

Conversation

@ClaudiaGivan
Copy link
Collaborator

No description provided.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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() and clearGraph() 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 clearGraph method in HistogramRenderer differs from other renderers by re-drawing the SVG and axes after clearing. This behavior is inconsistent with the pattern used in WorkItemAgeRenderer, MovingRangeRenderer, ControlRenderer, and the base Renderer class, where clearGraph only removes elements. The re-drawing logic should be in updateData or renderGraph instead. Consider removing lines 76-77 to maintain consistency.
  }


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ClaudiaGivan ClaudiaGivan force-pushed the renderers-data-update branch 4 times, most recently from 8fede10 to 4922fe0 Compare December 12, 2025 12:43
@ClaudiaGivan ClaudiaGivan merged commit 8c36723 into main Dec 12, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants