Add Team Lead Risk Overview UI - #821
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new Risk Overview dashboard for Team Leads, featuring a tabbed interface to monitor mentors, batches, and students at risk. The implementation includes design specifications, mock data, and client-side logic for sorting and filtering based on performance metrics. However, several critical issues were identified: the mentor table incorrectly applies data attributes to the header instead of the rows, breaking the sorting logic. Furthermore, the filter UI is incomplete, missing several controls referenced in the JavaScript, and the filtering logic itself relies on fragile string parsing that does not match the HTML structure. A typo in an SVG namespace URL was also noted.
| <tr data-risk-row data-tab="mentors" data-status="{{ mentor.status }}" | ||
| data-followup-coverage="{{ mentor.followup_coverage }}" data-mentor="{{ mentor.name }}"> |
There was a problem hiding this comment.
The data-risk-row attribute and associated data attributes are incorrectly applied to the <thead> row. This will cause the header to be included in the sorting and filtering logic in index.html, leading to potential errors or unexpected UI behavior.
<tr>
| </thead> | ||
| <tbody class="divide-y divide-stone-200 dark:divide-neutral-700"> | ||
| {% for mentor in team_lead_risk_overview.mentors %} | ||
| <tr> |
There was a problem hiding this comment.
The data rows in the mentors tab are missing the data-risk-row attribute and necessary metadata attributes. This prevents them from being correctly identified, sorted, or filtered by the script in index.html.
<tr data-risk-row data-tab="mentors" data-status="{{ mentor.status }}" data-followup-coverage="{{ mentor.followup_coverage }}" data-mentor="{{ mentor.name }}">
| <div class="flex flex-wrap justify-between items-center gap-3"> | ||
| <div class="grow flex flex-wrap items-center gap-2"> | ||
| <button id="risk-filter-at-risk-only" type="button" data-active="false" | ||
| class="hs-dropdown-toggle me-1 py-1 flex items-center gap-x-1 text-[13px] text-start text-stone-500 underline-offset-4 hover:underline disabled:opacity-50 disabled:pointer-events-none focus:outline-hidden focus:underline dark:text-neutral-500"> | ||
| <svg class="shrink-0 size-3.5" xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24" | ||
| fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round"> | ||
| <line x1="21" x2="14" y1="4" y2="4" /> | ||
| <line x1="10" x2="3" y1="4" y2="4" /> | ||
| <line x1="21" x2="12" y1="12" y2="12" /> | ||
| <line x1="8" x2="3" y1="12" y2="12" /> | ||
| <line x1="21" x2="16" y1="20" y2="20" /> | ||
| <line x1="12" x2="3" y1="20" y2="20" /> | ||
| <line x1="14" x2="14" y1="2" y2="6" /> | ||
| <line x1="8" x2="8" y1="10" y2="14" /> | ||
| <line x1="16" x2="16" y1="18" y2="22" /> | ||
| </svg> | ||
| Filters | ||
| </button> |
There was a problem hiding this comment.
The filter controls are incomplete compared to the design specification and the JavaScript implementation in index.html. Specifically, the search input and the dropdowns for filtering by Status, Reason, Mentor, and Batch are missing. The script currently references non-existent IDs like risk-filter-mentor and risk-filter-batch.
| const selectedMentor = defaultMentor; | ||
| const selectedBatch = defaultBatch; |
There was a problem hiding this comment.
The filtering logic for mentors and batches is currently non-functional because selectedMentor and selectedBatch are initialized as constants and never updated by user interaction. Furthermore, the event listeners for clearing these filters (lines 211-218) reference non-existent element IDs (risk-filter-mentor, risk-filter-batch).
| const parts = batchText.split(" - Mentor "); | ||
| if (parts.length === 2) batchToMentor.set(parts[0].trim(), parts[1].trim()); | ||
| } | ||
| }); | ||
|
|
||
| const mentorPillValueEl = document.getElementById("risk-pill-mentor-value"); | ||
| const batchPillValueEl = document.getElementById("risk-pill-batch-value"); | ||
| const reasonPillValueEl = document.getElementById("risk-pill-reason-value"); | ||
| const defaultMentor = Array.from(mentorSet).sort()[0] || mentorPillValueEl?.textContent?.trim() || ""; | ||
| const defaultBatch = Array.from(batchSet).sort()[0] || batchPillValueEl?.textContent?.trim() || ""; | ||
|
|
||
| function getActiveTabId() { | ||
| const tab = document.querySelector("button[role='tab'][aria-selected='true']"); | ||
| return tab?.id || "tabs-risk-mentors"; | ||
| } | ||
|
|
||
| function isAtRiskStatus(status) { | ||
| return status === "At Risk" || status === "Critical"; | ||
| } | ||
|
|
||
| function applyFilters() { | ||
| const atRiskOnly = atRiskBtn?.dataset.active === "true"; | ||
| const selectedMentor = defaultMentor; | ||
| const selectedBatch = defaultBatch; | ||
| const selectedReason = document.querySelector('[data-reason-chip-active="true"]')?.dataset.reason || reasonPillValueEl?.textContent?.trim() || ""; | ||
| const activeTabId = getActiveTabId(); | ||
|
|
||
| rows.forEach((row) => { | ||
| const tab = row.dataset.tab; | ||
| const status = row.dataset.status || ""; | ||
| const firstCellText = row.cells[0]?.innerText?.trim() || ""; | ||
| const rowReason = row.querySelector('[data-reason-chip]')?.dataset.reason || ""; | ||
| const isVisibleTab = | ||
| (activeTabId === "tabs-risk-mentors" && tab === "mentors") || | ||
| (activeTabId === "tabs-risk-batches" && tab === "batches") || | ||
| (activeTabId === "tabs-risk-students" && tab === "students"); | ||
|
|
||
| if (!isVisibleTab) { | ||
| row.style.display = ""; | ||
| return; | ||
| } | ||
|
|
||
| let show = true; | ||
| if (atRiskOnly && !isAtRiskStatus(status)) show = false; | ||
|
|
||
| if (tab === "students") { | ||
| const parts = firstCellText.split(" - Batch "); |
There was a problem hiding this comment.
The logic for extracting mentor and batch names from innerText using split(" - Mentor ") and split(" - Batch ") is fragile and will fail because the HTML structure does not contain these separator strings. It is recommended to use data attributes (e.g., data-mentor, data-batch) on the table rows to store this metadata and access it via dataset.
| "toggleClasses": "hs-select-disabled:pointer-events-none hs-select-disabled:opacity-50 relative py-1 ps-1.5 pe-7 inline-flex shrink-0 justify-center items-center gap-x-1.5 border border-transparent text-sm text-stone-500 dark:text-neutral-400 rounded-lg underline-offset-4 hover:underline hover:text-stone-600 dark:hover:text-neutral-300 focus:outline-hidden focus:underline focus:text-stone-600 dark:focus:text-neutral-300 before:absolute before:inset-0 before:z-1", | ||
| "dropdownClasses": "end-0 mt-2 p-1 z-20 w-64 bg-white dark:bg-neutral-900 border border-transparent rounded-xl shadow-xl", | ||
| "optionClasses": "flex items-center gap-x-3 py-2 px-3 text-xs sm:text-[13px] text-stone-800 dark:text-neutral-200 hover:bg-stone-100 dark:hover:bg-neutral-800 rounded-lg focus:outline-hidden focus:bg-stone-100 dark:focus:bg-neutral-800", | ||
| "optionTemplate": "<div class=\"flex justify-between items-center w-full\"><span data-title></span><span class=\"hidden hs-selected:block\"><svg class=\"shrink-0 size-3.5 text-stone-800 dark:text-neutral-200\" xmlns=\"http:.w3.org/2000/svg\" width=\"24\" height=\"24\" viewBox=\"0 0 24 24\" fill=\"none\" stroke=\"currentColor\" stroke-width=\"2\" stroke-linecap=\"round\" stroke-linejoin=\"round\"><polyline points=\"20 6 9 17 4 12\"/></svg></span></div>", |
There was a problem hiding this comment.
There is a typo in the SVG namespace URL (http:.w3 instead of http://www.w3).
| "optionTemplate": "<div class=\"flex justify-between items-center w-full\"><span data-title></span><span class=\"hidden hs-selected:block\"><svg class=\"shrink-0 size-3.5 text-stone-800 dark:text-neutral-200\" xmlns=\"http:.w3.org/2000/svg\" width=\"24\" height=\"24\" viewBox=\"0 0 24 24\" fill=\"none\" stroke=\"currentColor\" stroke-width=\"2\" stroke-linecap=\"round\" stroke-linejoin=\"round\"><polyline points=\"20 6 9 17 4 12\"/></svg></span></div>", | |
| "optionTemplate": "<div class=\"flex justify-between items-center w-full\"><span data-title></span><span class=\"hidden hs-selected:block\"><svg class=\"shrink-0 size-3.5 text-stone-800 dark:text-neutral-200\" xmlns=\"http://www.w3.org/2000/svg\" width=\"24\" height=\"24\" viewBox=\"0 0 24 24\" fill=\"none\" stroke=\"currentColor\" stroke-width=\"2\" stroke-linecap=\"round\" stroke-linejoin=\"round\"><polyline points=\"20 6 9 17 4 12\"/></svg></span></div>", |
|
62cb374 to
c509fb2
Compare
ee1c881 to
5cf4ce6
Compare
5cf4ce6 to
a5c5626
Compare
551e24b to
4fe1fdc
Compare
- Added a new Risk Overview page for team leads, featuring detailed insights on mentors, batches, and students at risk. - Created JSON data structure to support mentor and batch performance metrics, including at-risk status and follow-up coverage. - Developed HTML templates for displaying risk data, including tabs for mentors, batches, and students, with filtering options. - Enhanced CSS with new utility classes for improved layout and responsiveness. - Updated navigation to include a link to the new Risk Overview page.
0b92e64 to
1886d90
Compare
- Added new utility classes for minimum height and width adjustments in CSS. - Updated HTML templates to include follow-up modal and improved mentor and student display links. - Refactored table structure in mentors and students tabs for better layout and added action buttons for follow-ups. - Adjusted navigation links for consistency across the dashboard.
1 participant
No description provided.