Add Team Lead Risk Overview UI #821
Conversation
Capture Risk Overview, Batch Performance, and interaction flow decisions with clear scope boundaries so backend rules and APIs can be finalized later after UI validation.
Clarify At-Risk Students terminology, add critical-without-followup fallback action behavior, define segment filter persistence/reset, include batch risk insight line, and improve follow-up modal count clarity.
Add Risk Overview page with Mentors, Batches, and Students tabs using existing Simba table patterns, mock data in team_lead_risk_overview.json, filters and sort controls, reason tooltips, and header help popover for risk level rules. Link the page from the team lead menu.
Reformat filter pill defaults, update students tab markup, minor template tweaks.
![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>", |
|
…, padding, and dark mode styles. Added support for 30rem width, 44 min-width, 6 gap-x, and 1 padding-top. Introduced break-words and new placement and trigger utilities. Improved dark mode styles for text and backgrounds.
1 participant
No description provided.