Fix: Replace empty divs with proper Bootstrap modals for Respond to Call/Station#318
Conversation
…all/Station The commit RE1-T104 switched from Kendo UI Windows to Bootstrap modals but didn't add the required modal HTML structure. Kendo's kendoWindow() could auto-create the window on empty divs, but Bootstrap modals require actual HTML with .modal, .modal-dialog, .modal-content classes. This fix adds proper Bootstrap modal HTML for both respondToACallWindow and respondToAStationWindow modals in Dashboard.cshtml. Fixes Resgrid#317
📝 WalkthroughWalkthroughBootstrap modal structures were added to two placeholder container elements in the Dashboard view, replacing simple Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Web/Resgrid.Web/Areas/User/Views/Home/Dashboard.cshtml (1)
225-248: Good fix for the Bootstrap modal structure.The modal markup correctly provides the
.modal,.modal-dialog,.modal-content,.modal-header, and.modal-bodyhierarchy that Bootstrap requires. The empty.modal-bodyelements are intentional and work correctly with the JavaScriptshow.bs.modalhandlers that dynamically load content via.load().Minor accessibility improvement: Consider adding
aria-labelledbyattributes that reference the modal title, and give each<h4>anidfor proper screen reader association. This follows Bootstrap best practices for accessible modals.♿ Optional: Add accessibility attributes
-<div class="modal fade" tabindex="-1" role="dialog" id="respondToACallWindow"> +<div class="modal fade" tabindex="-1" role="dialog" aria-labelledby="respondToACallWindowLabel" aria-hidden="true" id="respondToACallWindow"> <div class="modal-dialog"> <div class="modal-content"> <div class="modal-header"> <button type="button" class="close" data-dismiss="modal" aria-hidden="true">×</button> - <h4 class="modal-title">@localizer["RespondToCall"]</h4> + <h4 class="modal-title" id="respondToACallWindowLabel">@localizer["RespondToCall"]</h4> </div> <div class="modal-body"> </div> </div> </div> </div> -<div class="modal fade" tabindex="-1" role="dialog" id="respondToAStationWindow"> +<div class="modal fade" tabindex="-1" role="dialog" aria-labelledby="respondToAStationWindowLabel" aria-hidden="true" id="respondToAStationWindow"> <div class="modal-dialog"> <div class="modal-content"> <div class="modal-header"> <button type="button" class="close" data-dismiss="modal" aria-hidden="true">×</button> - <h4 class="modal-title">@localizer["RespondToStation"]</h4> + <h4 class="modal-title" id="respondToAStationWindowLabel">@localizer["RespondToStation"]</h4> </div> <div class="modal-body"> </div> </div> </div> </div>Note: The existing
confirmSetAllToStandbyModalat line 206 has a similar gap—itsaria-labelledby="myModalLabel"references a non-existent id. You may want to fix that as well for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/Areas/User/Views/Home/Dashboard.cshtml` around lines 225 - 248, Add proper aria-labelledby attributes to the modal containers (respondToACallWindow and respondToAStationWindow) and assign matching unique ids to their title elements (<h4>), e.g., give the <h4> inside respondToACallWindow an id like "respondToACallLabel" and set aria-labelledby="respondToACallLabel" on the modal div; do the same for respondToAStationWindow. Also fix the existing confirmSetAllToStandbyModal by either creating an element with id "myModalLabel" or updating its aria-labelledby to reference the actual title id so screen readers correctly associate each modal with its heading.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Web/Resgrid.Web/Areas/User/Views/Home/Dashboard.cshtml`:
- Around line 225-248: Add proper aria-labelledby attributes to the modal
containers (respondToACallWindow and respondToAStationWindow) and assign
matching unique ids to their title elements (<h4>), e.g., give the <h4> inside
respondToACallWindow an id like "respondToACallLabel" and set
aria-labelledby="respondToACallLabel" on the modal div; do the same for
respondToAStationWindow. Also fix the existing confirmSetAllToStandbyModal by
either creating an element with id "myModalLabel" or updating its
aria-labelledby to reference the actual title id so screen readers correctly
associate each modal with its heading.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 55cf518d-4033-4b37-bed3-83b1bc68c80b
📒 Files selected for processing (1)
Web/Resgrid.Web/Areas/User/Views/Home/Dashboard.cshtml
Summary
This PR fixes the "Respond to a Call" functionality that was broken after the KendoUI removal in commit RE1-T104.
Problem
When clicking "Responding" → "Respond to a Call" (issue #317):
Root Cause
Commit RE1-T104 switched from Kendo UI Windows to Bootstrap modals but didn't add the required modal HTML structure:
kendoWindow()could auto-create the window on empty divs.modal,.modal-dialog,.modal-contentclasses<div id="respondToACallWindow"></div>had no modal structureFix
Added proper Bootstrap modal HTML structure for both:
respondToACallWindow- for responding to callsrespondToAStationWindow- for responding to stationsThe modal bodies are empty initially; content is loaded dynamically via JavaScript when the modal opens.
Testing
confirmSetAllToStandbyModalin the same file)show.bs.modalwill now find.modal-bodyelements to load content intoFixes #317
Summary by CodeRabbit