Refactor training program UI: CSS improvements and centralized JS

[devin-ai-integration]

Refactor training program UI: CSS improvements and centralized JS

Summary

This PR modernizes the training program UI by replacing Jinja2 inline calculations with CSS custom properties and converting custom modal implementations to native HTML <dialog> elements.

CSS Improvements:

• Score cells now use --score CSS variable with calc() for hue-based coloring instead of Jinja2 template calculations
• Progress bars use --pct CSS variable for dynamic width and color transitions

Dialog Modernization:

• Add Student dropdown converted to native <dialog> element
• Bulk Assign Task modal converted to native <dialog> element
• Removed manual backdrop/escape key handling (now handled natively)

JS Centralization:

• Created training_program.js for histogram modal functionality
• Templates now initialize CMS.TrainingProgram module with data instead of including inline JS fragment

Review & Testing Checklist for Human

• Test dialog functionality: Open/close the "Add Student" and "Bulk Assign Task" dialogs - verify they open with showModal(), close on X button, close on Cancel, and close on backdrop click
• Test score cell colors: View the combined ranking page and verify score cells display correct red→yellow→green gradient based on score values (0=red, 50=yellow, 100=green)
• Test progress bars: View the students page and verify progress bars show correct width and color based on percentage
• Test histogram modals: On combined ranking and training days pages, click histogram buttons and verify the modal opens with correct data and filtering works
• Browser compatibility: Test in target browsers - native <dialog> and CSS min()/max() require modern browser support

Recommended test plan:

1. Navigate to a training program's Students page
2. Click "Add Student" button - dialog should open as a modal
3. Navigate to Combined Ranking page
4. Verify score cells have colored backgrounds
5. Click on a task header to open histogram - verify it displays correctly

Notes

The fragments/histogram_js.html file was not deleted as it may be used elsewhere - the templates now use the centralized JS instead of including it.

Link to Devin run: https://app.devin.ai/sessions/b79fca6028144b089178e338ddf8f6f5
Requested by: Ron Ryvchin (@ronryv)

---

Summary by CodeRabbit

• New Features

  • Added password strength indicator for form inputs.
  • Enhanced table sorting and filtering capabilities.
  • Introduced histogram modal for score distribution analysis with tag-based filtering.
  • Added tag management with confirmation dialogs and server persistence.

• Improvements

  • Replaced modal dialogs with native HTML dialog elements.
  • Modernized progress bar visualization.
  • Improved score cell coloring based on performance metrics.
  • Enhanced date/time validation for form submissions.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment and CI monitoring

[coderabbitai]

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

• 🔍 Trigger a full review

📝 Walkthrough

Walkthrough

This PR refactors client-side utilities from a monolithic aws_utils.js file into three specialized modules (aws_form_utils.js, aws_table_utils.js, training_program.js), migrating histogram modal logic from an HTML fragment into a dedicated module. It modernizes UI components by replacing dropdown/modal patterns with native HTML dialogs and updates CSS styling for dialog chrome, progress indicators, and score-based cell coloring. Templates are updated to include new modules and initialize the training program module with configuration data, with backward-compatible aliasing maintained for existing API calls.

Changes

Cohort / File(s)	Summary
New Form Utilities Module cms/server/admin/static/aws_form_utils.js	Introduces 5 exported functions: password strength indicator with zxcvbn, datetime validation with start/end time checking, remove-page workflow with optional task targeting, read-only Tagify tags display, and advanced Tagify tag management with confirmation flows and server persistence. Includes backward-compatible aliases to CMS.AWSUtils on DOMContentLoaded.
New Table Utilities Module cms/server/admin/static/aws_table_utils.js	Adds 4 exported functions: row comparator for numeric/string sorting, stable two-pass table sorting with optional ranks column re-numbering, clickable header initialization for sortable tables, and case-insensitive row filtering. Includes backward-compatible aliases to CMS.AWSUtils (sort_table, init_table_sort, filter_table).
New Training Program Module cms/server/admin/static/training_program.js	Centralizes histogram modal UI with tag-based filtering, score bucketing, statistics computation (total, average, median, max, min), and clipboard export. Provides init(options) for configuration, openHistogramModal/closeHistogramModal for modal control, copyHistogramData for data export. Maintains module-scoped state for modal, Tagify instance, and configuration maps. Exposes global wrappers for backward compatibility.
Refactored Utilities cms/server/admin/static/aws_utils.js	Removes 9 methods that were moved to new modules (sort_table, init_table_sort, filter_table, initPasswordStrength, initDateTimeValidation, initRemovePage, initReadOnlyTagify, initTagify, get_table_row_comparator). Core utilities module now focuses on remaining functionality.
Updated Dialog & Progress Styling cms/server/admin/static/aws_tp_styles.css	Replaces Add Student dropdown with native dialog element styles (.tp-dialog, .tp-dialog-header, .tp-dialog-footer, .tp-dialog-close). Introduces modern progress bar styling (.progress-bar-modern) and score-based cell coloring (.cell-content.score-cell with --score CSS variable). Removes legacy Year Progress block styles and inline progress bar implementations.
Template Includes & Fragment Removal cms/server/admin/templates/base.html, cms/server/admin/templates/fragments/histogram_js.html	Adds script includes for aws_table_utils.js, aws_form_utils.js, and training_program.js in base template. Removes entire 361-line histogram_js.html fragment (initHistogramModal, openHistogramModal, renderHistogram, copyHistogramData, and associated state/event handlers) as functionality is now in training_program.js.
Training Program Template Updates cms/server/admin/templates/training_program_combined_ranking.html, cms/server/admin/templates/training_program_training_days.html	Replaces histogram_js.html fragment includes with inline CMS.TrainingProgram.init(...) calls, passing allStudentTags, tagsPerTrainingDay, historicalStudentTags, studentData, trainingDayTasks, studentAccessibleTasks, and taskMaxScores/taskMaxScoresByTrainingDay. Removes inline hue-based color calculations from render_score_cell, using CSS --score variable instead.
Student Training Program & Dialog Migration cms/server/admin/templates/training_program_students.html	Replaces Add Student dropdown form and Bulk Assign Task modal with native HTML dialog elements (add-student-dialog, bulkAssignDialog). Updates progress display from inline bars to CSS-based modern representation (progress-link, progress-bar-modern, progress-score classes). Renames modal functions to dialog equivalents (initBulkAssignModal → initBulkAssignDialog, etc.).
Minor Template Restructuring cms/server/admin/templates/ranking.html, cms/server/admin/templates/training_day_groups.html	Updates checkbox selector from id="show-indicators" to class="show-indicators-toggle" with corresponding jQuery binding updates. Introduces has_groups conditional variable to wrap main groups table rendering and form controls, preserving table header while conditionally showing rows and Save button.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• Add training day types, student tag filtering, and score histograms #96: Overlapping training-day types, student-tag filtering, and histogram modal functionality directly shared with this refactoring.
• Redesign tasks page and add scoreboard sharing for archived training days #107: Modifies identical files (training_program.js, aws_tp_styles.css, histogram templates) for histogram/modal UI updates.
• Add TrainingDay model and admin interface for managing training days #55: Refactors frontend utilities (Tagify, validation, table sorting) in parallel with this PR's approach to modularization.

Suggested reviewers

• Daniel-Aga
• Command-Master

Poem

🐰 Hop, hop! The code now splits so clean,
Three modules where one module had been,
Forms and tables, histograms bright,
Dialogs native—what a delight!
Backward-compatible, alias so true,
The refactor's done, and it works brand new! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Refactor training program UI: CSS improvements and centralized JS' directly and clearly summarizes the main changes in the pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[devin-ai-integration]

Devin Review found 1 potential issue.

View issue and 5 additional flags in Devin Review.

[devin-ai-integration]

Devin Review found 1 new potential issue.

View issue and 9 additional flags in Devin Review.

[devin-ai-integration]

Devin Review found 2 new potential issues.

View issues and 10 additional flags in Devin Review.

[devin-ai-integration]

Devin Review found 1 new potential issue.

View issue and 12 additional flags in Devin Review.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@cms/server/admin/static/aws_table_utils.js`:
- Around line 11-12: The file initializes the CMS namespace using a
self-referential var ("var CMS = CMS || {};" ) which triggers Biome's
noInvalidUseBeforeDeclaration; replace that line with a safe global
initialization such as using globalThis (e.g. globalThis.CMS = globalThis.CMS ||
{}) or, if you only target browsers, window.CMS = window.CMS || {}; then keep
the existing CMS.AWSTableUtils assignment (CMS.AWSTableUtils = CMS.AWSTableUtils
|| {}); alternatively, if you must keep the legacy pattern, add a Biome
suppression comment (biome-ignore noInvalidUseBeforeDeclaration) immediately
above the CMS initialization to silence the rule.

🧹 Nitpick comments (1)

cms/server/admin/static/aws_form_utils.js (1)

383-387: Consider documenting the armed delay rationale.

The 100ms delay before arming confirmations prevents spurious dialogs during initial page load when Tagify processes existing tag values. While functional, a brief comment explaining this timing would aid future maintainers.

         // Arm the confirmations after a short delay to skip initial load events
+        // The 100ms delay allows Tagify to finish processing pre-existing tags
+        // before we start showing confirmation dialogs for user-initiated changes
         setTimeout(function() {
             armed = true;
         }, 100);

[coderabbitai]

Actionable comments posted: 3

🤖 Fix all issues with AI agents

In `@cms/server/admin/static/aws_form_utils.js`:
- Around line 101-121: The target dropdown is always disabled on load; update
CMS.AWSFormUtils.initRemovePage to initialize targetSelectEl.disabled based on
the current action radio state instead of always setting it to true: after
caching targetSelectEl and moveRadioEl (and after the early return check) set
targetSelectEl.disabled = !moveRadioEl.checked so a pre-selected "move" leaves
the dropdown enabled; keep the existing change event handler on
input[name="action"] to handle subsequent toggles.
- Around line 61-87: The submit handler in
CMS.AWSFormUtils.initDateTimeValidation currently uses truthy checks for
startValue and stopValue which incorrectly treats 0 as invalid; change the
conditional to explicitly check for valid numbers using !isNaN(startValue) &&
!isNaN(stopValue) before comparing stopValue <= startValue, so the validation
runs for epoch (0) and all other valid datetimes while still skipping when
either value is NaN or empty.

In `@cms/server/admin/static/training_program.js`:
- Around line 8-16: The CMS bootstrap uses "var CMS = CMS || {};" which reads
CMS before declaration and triggers Biome's noInvalidUseBeforeDeclaration;
change it to reference the global object explicitly (e.g. globalThis.CMS =
globalThis.CMS || {} or window.CMS = window.CMS || {} depending on environment)
and then initialize CMS.TrainingProgram = CMS.TrainingProgram || {}; so update
the top-level initialization in this file (the CMS bootstrap lines) to use the
globalThis/window global reference to avoid the hoisting/read-before-declare
issue while keeping the CMS.TrainingProgram setup unchanged.

🧹 Nitpick comments (1)

cms/server/admin/templates/training_program_students.html (1)

220-271: Consider initializing TomSelect after dialog is shown.

TomSelect (via .searchable-select) is initialized in base.html's init() function on page load. However, TomSelect may have sizing issues when initialized on elements inside a closed dialog (since the element has no computed dimensions).

💡 Optional: Initialize TomSelect when dialog opens

If users report dropdown issues in the Add Student dialog, consider initializing TomSelect after the dialog is shown:

// In the onclick handler or a dedicated function
document.getElementById('add-student-dialog').addEventListener('shown', function() {
    var select = this.querySelector('.searchable-select:not(.tomselected)');
    if (select) new TomSelect(select, { /* options */ });
});

However, if testing shows no issues, the current approach is acceptable since TomSelect handles hidden elements reasonably well.

[ronryv]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In `@cms/server/admin/static/aws_table_utils.js`:
- Around line 11-12: The module assigns to CMS.AWSTableUtils without a local CMS
binding which throws in strict mode; update the top of the file to establish a
local CMS variable like the pattern in aws_form_utils.js (e.g. create a local
binding by assigning CMS = window.CMS = window.CMS || {}), then set
CMS.AWSTableUtils = CMS.AWSTableUtils || {}; so references to CMS and
CMS.AWSTableUtils (and any subsequent uses in this file) use the local binding
and avoid a ReferenceError.

In `@cms/server/admin/templates/training_program_students.html`:
- Around line 42-78: The dialog uses the native dialog element with direct calls
to showModal() and close() (see the element id "add-student-dialog" and the
inline onclick handlers that call
document.getElementById('add-student-dialog').showModal() / .close()); add
feature detection or a polyfill: detect whether HTMLDialogElement and showModal
are supported before calling them and provide a graceful fallback (e.g., toggle
a CSS modal class or load a dialog polyfill library) so older browsers (IE,
older Safari/Chrome/Firefox) don’t throw errors when opening/closing the
add-student-dialog.

🧹 Nitpick comments (2)

cms/server/admin/templates/fragments/training_day_groups.html (2)

90-92: Consider consistent naming convention for form inputs.

The checkbox uses name="alphabetical_{{ group.id }}" while other inputs use array notation (group_id[], start_time[], etc.). This inconsistency may require special handling on the server side to associate the checkbox with the correct group.

If intentional for server-side logic, this is fine. Otherwise, consider using a consistent pattern.

💡 Optional: Use array notation with index for consistency

-        <input type="checkbox" name="alphabetical_{{ group.id }}" {{ "checked" if group.alphabetical_task_order else "" }}/>
+        <input type="checkbox" name="alphabetical[]" value="{{ group.id }}" {{ "checked" if group.alphabetical_task_order else "" }}/>

This would send only checked group IDs in the alphabetical[] array, which the server can then match against group_id[].

---

42-53: Consider adding a timeout for the fetch request.

The removeMainGroup function's fetch call lacks a timeout, which could leave the user waiting indefinitely if the server is unresponsive. For an admin tool, this is a minor concern but worth noting.

💡 Optional: Add AbortController timeout

 function removeMainGroup(url) {
     if (!confirm('Remove this main group?')) {
         return;
     }
     var xsrfInput = document.querySelector('input[name="_xsrf"]');
     if (!xsrfInput) {
         alert('Missing XSRF token');
         return;
     }
+    var controller = new AbortController();
+    var timeoutId = setTimeout(function() { controller.abort(); }, 30000);
     var formData = new FormData();
     formData.append('_xsrf', xsrfInput.value);
     fetch(url, {
         method: 'POST',
-        body: formData
+        body: formData,
+        signal: controller.signal
     }).then(function(response) {
+        clearTimeout(timeoutId);
         if (response.ok) {
             window.location.reload();
         } else {
             alert('Failed to remove main group');
         }
     }).catch(function(error) {
+        clearTimeout(timeoutId);
-        alert('Error: ' + error);
+        if (error.name === 'AbortError') {
+            alert('Request timed out');
+        } else {
+            alert('Error: ' + error);
+        }
     });
 }

[sonarqubecloud]

Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE