Refactor training program UI: CSS improvements and centralized JS#116
Merged
ronryv merged 9 commits intotraining_programfrom Jan 31, 2026
Merged
Conversation
- Replace Jinja2 inline color calculations with CSS custom properties - Score cells now use CSS calc() with --score variable for hue calculation - Progress bars use CSS custom properties for dynamic width and color - Replace custom dropdown with native HTML <dialog> element - Add student dialog uses native dialog for better accessibility - Bulk assign modal converted to native dialog element - Removed manual backdrop/escape key handling (native behavior) - Create centralized training_program.js for histogram functionality - Move histogram modal logic from inline template to dedicated JS file - Templates now initialize CMS.TrainingProgram module with data - Maintains backwards compatibility with existing onclick handlers - Add new CSS classes for dialog and progress bar components - .tp-dialog, .tp-dialog-header, .tp-dialog-body, .tp-dialog-footer - .progress-link, .progress-percentage, .progress-score - .cell-content.score-cell for CSS-based score coloring - .progress-bar-modern for CSS-based progress bars Co-Authored-By: Ron Ryvchin <ron.ryv@gmail.com>
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the
📝 WalkthroughWalkthroughThis PR refactors client-side utilities from a monolithic Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
- Remove countdown timer from training program footer (keep only for contests) - Fix duplicate 'main-groups-table' ID in training_day_groups.html by consolidating table structure - Fix duplicate 'show-indicators' ID in ranking.html by using class selector - Create aws_table_utils.js with table sorting/filtering functions - Create aws_form_utils.js with form utilities (tagify, password, datetime, remove page) - Update base.html to include new JS files - Refactor training_program.js to use namespace pattern (CMS.TrainingProgram) Co-Authored-By: Ron Ryvchin <ron.ryv@gmail.com>
- Add overflow: visible to tp-dialog to allow dropdown to extend beyond dialog - Add min-height to dialog body to ensure buttons are visible when dropdown is open Co-Authored-By: Ron Ryvchin <ron.ryv@gmail.com>
Functions moved to aws_table_utils.js: - get_table_row_comparator - sort_table - init_table_sort - filter_table Functions moved to aws_form_utils.js: - initPasswordStrength - initDateTimeValidation - initRemovePage - initReadOnlyTagify - initTagify This completes the modularization of aws_utils.js, reducing it from 1728 lines to 1179 lines while maintaining backward compatibility through aliases in the new utility files. Co-Authored-By: Ron Ryvchin <ron.ryv@gmail.com>
ronryv
force-pushed
the
devin/1769876720-refactor-training-program-js
branch
from
ea34c8b to
abe0db2
Compare
ronryv
force-pushed
the
devin/1769876720-refactor-training-program-js
branch
from
abe0db2 to
c93d204
Compare
This file contained histogram modal JavaScript that was duplicated in training_program.js during the refactoring. The histogram functionality is now centralized in training_program.js, making this file obsolete. Co-Authored-By: Ron Ryvchin <ron.ryv@gmail.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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);
There was a problem hiding this comment.
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 inbase.html'sinit()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.
Collaborator
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
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 againstgroup_id[].
42-53: Consider adding a timeout for the fetch request.The
removeMainGroupfunction'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); + } }); }
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
ronryv
added a commit
that referenced
this pull request
Feb 2, 2026
* Refactor training program UI: CSS improvements and centralized JS - Replace Jinja2 inline color calculations with CSS custom properties - Score cells now use CSS calc() with --score variable for hue calculation - Progress bars use CSS custom properties for dynamic width and color - Replace custom dropdown with native HTML <dialog> element - Add student dialog uses native dialog for better accessibility - Bulk assign modal converted to native dialog element - Removed manual backdrop/escape key handling (native behavior) - Create centralized training_program.js for histogram functionality - Move histogram modal logic from inline template to dedicated JS file - Templates now initialize CMS.TrainingProgram module with data - Maintains backwards compatibility with existing onclick handlers - Add new CSS classes for dialog and progress bar components - .tp-dialog, .tp-dialog-header, .tp-dialog-body, .tp-dialog-footer - .progress-link, .progress-percentage, .progress-score - .cell-content.score-cell for CSS-based score coloring - .progress-bar-modern for CSS-based progress bars Co-Authored-By: Ron Ryvchin <ron.ryv@gmail.com> * Fix duplicate HTML IDs and split aws_utils.js into modules - Remove countdown timer from training program footer (keep only for contests) - Fix duplicate 'main-groups-table' ID in training_day_groups.html by consolidating table structure - Fix duplicate 'show-indicators' ID in ranking.html by using class selector - Create aws_table_utils.js with table sorting/filtering functions - Create aws_form_utils.js with form utilities (tagify, password, datetime, remove page) - Update base.html to include new JS files - Refactor training_program.js to use namespace pattern (CMS.TrainingProgram) Co-Authored-By: Ron Ryvchin <ron.ryv@gmail.com> * Fix Add Student dialog size to accommodate dropdown - Add overflow: visible to tp-dialog to allow dropdown to extend beyond dialog - Add min-height to dialog body to ensure buttons are visible when dropdown is open Co-Authored-By: Ron Ryvchin <ron.ryv@gmail.com> * Remove extracted functions from aws_utils.js (cleanup) Functions moved to aws_table_utils.js: - get_table_row_comparator - sort_table - init_table_sort - filter_table Functions moved to aws_form_utils.js: - initPasswordStrength - initDateTimeValidation - initRemovePage - initReadOnlyTagify - initTagify This completes the modularization of aws_utils.js, reducing it from 1728 lines to 1179 lines while maintaining backward compatibility through aliases in the new utility files. Co-Authored-By: Ron Ryvchin <ron.ryv@gmail.com> * cleanup css * Remove unused histogram_js.html fragment This file contained histogram modal JavaScript that was duplicated in training_program.js during the refactoring. The histogram functionality is now centralized in training_program.js, making this file obsolete. Co-Authored-By: Ron Ryvchin <ron.ryv@gmail.com> * nits * nits * Update cms/server/admin/static/aws_table_utils.js Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: Ron Ryvchin <ron.ryv@gmail.com> Co-authored-by: ronryv <57587851+ronryv@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
ronryv
added a commit
that referenced
this pull request
Feb 2, 2026
* Refactor training program UI: CSS improvements and centralized JS - Replace Jinja2 inline color calculations with CSS custom properties - Score cells now use CSS calc() with --score variable for hue calculation - Progress bars use CSS custom properties for dynamic width and color - Replace custom dropdown with native HTML <dialog> element - Add student dialog uses native dialog for better accessibility - Bulk assign modal converted to native dialog element - Removed manual backdrop/escape key handling (native behavior) - Create centralized training_program.js for histogram functionality - Move histogram modal logic from inline template to dedicated JS file - Templates now initialize CMS.TrainingProgram module with data - Maintains backwards compatibility with existing onclick handlers - Add new CSS classes for dialog and progress bar components - .tp-dialog, .tp-dialog-header, .tp-dialog-body, .tp-dialog-footer - .progress-link, .progress-percentage, .progress-score - .cell-content.score-cell for CSS-based score coloring - .progress-bar-modern for CSS-based progress bars Co-Authored-By: Ron Ryvchin <ron.ryv@gmail.com> * Fix duplicate HTML IDs and split aws_utils.js into modules - Remove countdown timer from training program footer (keep only for contests) - Fix duplicate 'main-groups-table' ID in training_day_groups.html by consolidating table structure - Fix duplicate 'show-indicators' ID in ranking.html by using class selector - Create aws_table_utils.js with table sorting/filtering functions - Create aws_form_utils.js with form utilities (tagify, password, datetime, remove page) - Update base.html to include new JS files - Refactor training_program.js to use namespace pattern (CMS.TrainingProgram) Co-Authored-By: Ron Ryvchin <ron.ryv@gmail.com> * Fix Add Student dialog size to accommodate dropdown - Add overflow: visible to tp-dialog to allow dropdown to extend beyond dialog - Add min-height to dialog body to ensure buttons are visible when dropdown is open Co-Authored-By: Ron Ryvchin <ron.ryv@gmail.com> * Remove extracted functions from aws_utils.js (cleanup) Functions moved to aws_table_utils.js: - get_table_row_comparator - sort_table - init_table_sort - filter_table Functions moved to aws_form_utils.js: - initPasswordStrength - initDateTimeValidation - initRemovePage - initReadOnlyTagify - initTagify This completes the modularization of aws_utils.js, reducing it from 1728 lines to 1179 lines while maintaining backward compatibility through aliases in the new utility files. Co-Authored-By: Ron Ryvchin <ron.ryv@gmail.com> * cleanup css * Remove unused histogram_js.html fragment This file contained histogram modal JavaScript that was duplicated in training_program.js during the refactoring. The histogram functionality is now centralized in training_program.js, making this file obsolete. Co-Authored-By: Ron Ryvchin <ron.ryv@gmail.com> * nits * nits * Update cms/server/admin/static/aws_table_utils.js Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: Ron Ryvchin <ron.ryv@gmail.com> Co-authored-by: ronryv <57587851+ronryv@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
This was referenced Feb 3, 2026
Draft
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
1 participant
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.
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:
--scoreCSS variable withcalc()for hue-based coloring instead of Jinja2 template calculations--pctCSS variable for dynamic width and color transitionsDialog Modernization:
<dialog>element<dialog>elementJS Centralization:
training_program.jsfor histogram modal functionalityCMS.TrainingProgrammodule with data instead of including inline JS fragmentReview & Testing Checklist for Human
showModal(), close on X button, close on Cancel, and close on backdrop click<dialog>and CSSmin()/max()require modern browser supportRecommended test plan:
Notes
The
fragments/histogram_js.htmlfile 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
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.