Skip to content

Staging into Main - fix charts display - #209

Merged
rsmoke merged 11 commits into
mainfrom
staging
Feb 25, 2026
Merged

Staging into Main - fix charts display#209
rsmoke merged 11 commits into
mainfrom
staging

Conversation

@rsmoke

@rsmoke rsmoke commented Feb 25, 2026

Copy link
Copy Markdown
Member

This pull request focuses on improving the management of Chart.js instances for questionnaire response charts, especially in the context of Turbo navigation, to prevent memory leaks and ensure charts are properly initialized and destroyed. Additionally, there is a minor optimization in the loading of faculty associations in the student calendar controller.

Chart.js instance management and Turbo navigation improvements:

  • Added global event listeners in app/javascript/application.js to destroy all questionnaireCharts Chart.js instances before Turbo navigates away, and to re-initialize charts on page load or Turbo load, preventing memory leaks and duplicate charts.
  • Refactored the chart initialization logic in app/views/questionnaires/responses.html.erb to:
    • Use a global window.questionnaireCharts array to track chart instances.
    • Destroy existing charts before creating new ones, both on initial load and when revisiting via Turbo.
    • Store newly created chart instances in the global array for easy cleanup.

Student calendar controller optimization:

  • Updated .includes in app/controllers/student/calendar_controller.rb to preload the :participating_faculty association along with calendar_event_faculties: :vip, reducing N+1 queries and improving performance.

dependabot Bot and others added 10 commits February 18, 2026 02:54
Bumps [rspec-rails](https://github.com/rspec/rspec-rails) from 8.0.2 to 8.0.3.
- [Changelog](https://github.com/rspec/rspec-rails/blob/main/Changelog.md)
- [Commits](rspec/rspec-rails@v8.0.2...v8.0.3)

---
updated-dependencies:
- dependency-name: rspec-rails
  dependency-version: 8.0.3
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [selenium-webdriver](https://github.com/SeleniumHQ/selenium) from 4.40.0 to 4.41.0.
- [Release notes](https://github.com/SeleniumHQ/selenium/releases)
- [Changelog](https://github.com/SeleniumHQ/selenium/blob/trunk/rb/CHANGES)
- [Commits](SeleniumHQ/selenium@selenium-4.40.0...selenium-4.41.0)

---
updated-dependencies:
- dependency-name: selenium-webdriver
  dependency-version: 4.41.0
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [solid_queue](https://github.com/rails/solid_queue) from 1.3.1 to 1.3.2.
- [Release notes](https://github.com/rails/solid_queue/releases)
- [Commits](rails/solid_queue@v1.3.1...v1.3.2)

---
updated-dependencies:
- dependency-name: solid_queue
  dependency-version: 1.3.2
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [web-console](https://github.com/rails/web-console) from 4.2.1 to 4.3.0.
- [Release notes](https://github.com/rails/web-console/releases)
- [Changelog](https://github.com/rails/web-console/blob/main/CHANGELOG.markdown)
- [Commits](rails/web-console@v4.2.1...v4.3.0)

---
updated-dependencies:
- dependency-name: web-console
  dependency-version: 4.3.0
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
….0.3

Bump rspec-rails from 8.0.2 to 8.0.3
…river-4.41.0

Bump selenium-webdriver from 4.40.0 to 4.41.0
….3.2

Bump solid_queue from 1.3.1 to 1.3.2
….3.0

Bump web-console from 4.2.1 to 4.3.0
- Added event listeners to destroy existing Chart.js instances before Turbo navigates away, preventing memory leaks.
- Implemented initialization of questionnaire charts on DOM content load and Turbo load events.
- Updated the responses view to ensure proper destruction of charts when re-initializing, improving performance and reliability.
- Updated the calendar controller to include participating faculty in event queries, improving data retrieval efficiency.
- Adjusted multiple instances of event loading to ensure consistency across the application.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR aims to fix Questionnaire response charts (Chart.js) under Turbo navigation by tracking/destroying chart instances across visits, and also reduces N+1 queries in the student calendar by preloading faculty associations.

Changes:

  • Added global Turbo lifecycle handlers to destroy and (re)initialize questionnaire charts across Turbo navigations.
  • Refactored questionnaire responses view JS to track Chart.js instances in a global array and destroy existing instances before recreating.
  • Optimized student calendar event loading by preloading :participating_faculty.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.

File Description
app/views/questionnaires/responses.html.erb Defines initQuestionnaireCharts() and tracks/destroys per-question Chart.js instances via window.questionnaireCharts.
app/javascript/application.js Adds Turbo event listeners to destroy charts on navigation and call initQuestionnaireCharts() on load events.
app/controllers/student/calendar_controller.rb Expands .includes to preload :participating_faculty to reduce N+1s.
Gemfile.lock Updates various gem versions as part of the staging-to-main merge.

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

window.questionnaireCharts = [];
}
});

Copilot AI Feb 25, 2026

Copy link

Choose a reason for hiding this comment

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

Using turbo:before-render to tear down Chart.js instances may be too late for Turbo’s page snapshot caching; the recommended hook for cleanup is turbo:before-cache so the cached DOM doesn’t retain canvas/chart state and Chart instances don’t linger in memory. Consider moving (or duplicating) this teardown to turbo:before-cache (and keeping before-render only if you have a specific need for it).

Suggested change
// Also destroy questionnaire Chart.js instances before Turbo caches the page snapshot
document.addEventListener("turbo:before-cache", function () {
if (window.questionnaireCharts && Array.isArray(window.questionnaireCharts)) {
window.questionnaireCharts.forEach(function (chart) {
try {
if (chart && typeof chart.destroy === "function") chart.destroy();
} catch (e) {
console.warn("Questionnaire chart destroy:", e);
}
});
window.questionnaireCharts = [];
}
});

Copilot uses AI. Check for mistakes.
Comment thread app/javascript/application.js Outdated
- Consolidated the initialization of questionnaire charts into a single function, improving code clarity and maintainability.
- Updated event listeners to ensure charts are initialized on both DOMContentLoaded and Turbo load events, enhancing responsiveness.
- Exposed the initQuestionnaireCharts function in the responses view for better accessibility.
@rsmoke
rsmoke merged commit 2b46b72 into main Feb 25, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants