Skip to content

feat(plan): tap/focus disclosure for the "why" reason, not just hover #4349

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
chalfontchubby wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat(plan): tap/focus disclosure for the "why" reason, not just hover #4349

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f0eaa64
feat(plan): tap/focus disclosure for the "why" reason, not just hover
chalfontchubby Jul 26, 2026
8b56d39
fix(plan): whitelist the reason cell in the click-outside dropdown cl…
chalfontchubby Jul 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 50 additions & 9 deletions apps/predbat/tests/test_plan_why_reason.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,21 +343,62 @@ def render():
if "title=" not in renderer_js:
print("ERROR: expected renderStateCell to emit a title= attribute")
failed = True
if "state2_color || '#FFFFFF'}${titleAttr}" not in renderer_js:
print("ERROR: expected the split (state2) cell to also carry the title= tooltip, not just the first half")
failed = True

# --- Test 13: the read-only (History / Yesterday Without Predbat) state cells also get tooltips ---
# Scoped to renderStateCell's own body (not a whole-file count) since the read-only path
# (Test 13 below) has its own, separate titleAttr usage with the same variable name.
state_cell_fn_start = renderer_js.index("function renderStateCell(")
state_cell_fn_end = renderer_js.index("function renderRateCell(", state_cell_fn_start)
state_cell_fn_src = renderer_js[state_cell_fn_start:state_cell_fn_end]
if state_cell_fn_src.count("${titleAttr}") != 2:
print("ERROR: expected both the first and split (state2) cells to carry the title= tooltip, found {} references".format(state_cell_fn_src.count("${titleAttr}")))
failed = True

# --- Test 13a: tap/focus disclosure for touch and keyboard users, alongside title= for mouse
# hover - a touch interaction can't trigger :hover/title at all, so the two mechanisms never
# fire together and there's nothing to reconcile between them (see PR discussion). ---
print("Test renderStateCell also wires a tap/focus disclosure panel, not just hover")
if "toggleForceDropdown" not in renderer_js:
print("ERROR: expected renderStateCell to reuse the existing toggleForceDropdown() tap-to-toggle mechanism")
failed = True
if "clickable-state-cell" not in renderer_js:
print("ERROR: expected renderStateCell to mark reason cells with the clickable-state-cell class")
failed = True
if "reason-text" not in renderer_js:
print("ERROR: expected renderStateCell to render the reason panel content with the reason-text class")
failed = True
if renderer_js.count("dropdown-content") < 2: # at least the CSS rule plus renderStateCell's own usage
print("ERROR: expected renderStateCell to reuse the existing .dropdown-content panel mechanism")
failed = True
if 'tabindex="0"' not in renderer_js or "onkeydown" not in renderer_js:
print("ERROR: expected the reason cell to be keyboard-focusable and keyboard-operable, not just tap/click - a bare <td onclick> isn't reachable by keyboard")
failed = True
# Regression: the document-level "click outside closes the dropdown" handler (in get_plan_css(),
# a separate function from get_plan_renderer_js()) only whitelisted .clickable-time-cell, so
# opening the new reason panel (class clickable-state-cell) would have its own click event
# immediately re-close it via that same handler (Copilot review on #4349).
plan_css = web_helper.get_plan_css()
outside_click_start = plan_css.index("Close dropdowns when clicking outside")
outside_click_src = plan_css[outside_click_start : outside_click_start + 400]
if ".clickable-state-cell" not in outside_click_src:
print("ERROR: the click-outside handler doesn't whitelist .clickable-state-cell, so tapping a reason cell would open then immediately close its own panel")
failed = True

# --- Test 13b: the read-only (History / Yesterday Without Predbat) state cells also get tooltips ---
print("Test the non-editable state cell path also emits a title= tooltip")
if "function reasonTitleAttr" not in renderer_js:
print("ERROR: expected a shared reasonTitleAttr() helper used by both state-cell paths")
failed = True
if "state_color || '#FFFFFF'}${titleAttr}" not in renderer_js:
# Scoped to the read-only branch specifically (between its own reasonTitleAttr() call and the
# next cell section) - the editable path (renderStateCell, Test 12 above) builds its split-cell
# HTML differently (extra tap/focus attributes sit between the bgcolor and titleAttr), so a
# single literal substring can no longer match both paths' shapes.
readonly_branch_start = renderer_js.index("const titleAttr = reasonTitleAttr(row, reasonTemplates);")
readonly_branch_end = renderer_js.index("// Limit cell", readonly_branch_start)
readonly_branch_src = renderer_js[readonly_branch_start:readonly_branch_end]
if "state_color || '#FFFFFF'}${titleAttr}" not in readonly_branch_src:
print("ERROR: expected the read-only state cell (editable=false) to carry the title= tooltip")
failed = True
# Both the editable and read-only paths render a split second half, so the tooltip appears twice
if renderer_js.count("state2_color || '#FFFFFF'}${titleAttr}") != 2:
print("ERROR: expected both the editable and read-only split cells to carry the title= tooltip")
if "state2_color || '#FFFFFF'}${titleAttr}" not in readonly_branch_src:
print("ERROR: expected the read-only split (state2) cell to carry the title= tooltip")
failed = True
# Templates must come from the dataset being rendered, not the plan view's global - otherwise
# the History/Yesterday views would render against the wrong (or a missing) template table
Expand Down
71 changes: 67 additions & 4 deletions apps/predbat/web_helper.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5697,6 +5697,41 @@ def get_plan_css():
z-index: 2000;
}

.clickable-state-cell {
cursor: pointer;
position: relative;
transition: background-color 0.2s;
z-index: 1;
}

.clickable-state-cell:has(.dropdown-content[style*="display: block"]) {
z-index: 2000;
}

.clickable-state-cell:hover {
filter: brightness(0.9);
}

.clickable-state-cell:focus-visible {
outline: 2px solid #2196F3;
outline-offset: -2px;
}

body.dark-mode .clickable-state-cell:hover {
filter: brightness(1.2);
}

.reason-text {
font-size: 13px;
line-height: 1.4;
color: #333;
max-width: 260px;
}

body.dark-mode .reason-text {
color: #eee;
}

.clickable-time-cell:hover {
filter: brightness(0.9);
}
Expand Down Expand Up @@ -6313,7 +6348,7 @@ def get_plan_css():

// Close dropdowns when clicking outside
document.addEventListener("click", function(event) {
if (!event.target.matches('.clickable-time-cell') && !event.target.closest('.dropdown-content')) {
if (!event.target.matches('.clickable-time-cell') && !event.target.matches('.clickable-state-cell') && !event.target.closest('.dropdown-content')) {
closeDropdowns();
}
});
Expand Down Expand Up @@ -6755,17 +6790,45 @@ def get_plan_renderer_js():

const rowspanAttr = row.rowspan_state > 0 ? ` rowspan="${row.rowspan_state}"` : '';
const colspanAttr = row.split ? '' : ' colspan=2';
const titleAttr = reasonTitleAttr(row, templates);
// reasonText is needed raw (not just as a title= attribute) for reasonCellAttrs() below,
// which also uses it for the tap/focus panel content - templates comes from the dataset
// being rendered (jsonData.reason_templates), not window.planData, so History/Yesterday
// views look up against their own template table rather than the plan view's.
const reasonText = renderReasonText(row.reasons, templates);
// Keep both title= (free instant hover for desktop/mouse) and the tap/focus panel below
// (for touch and keyboard, neither of which can trigger a hover state at all) - the two
// never fire together in practice, since a touch interaction can't trigger :hover/title
// in the first place, so there's nothing to reconcile between them.
const titleAttr = reasonText ? ` title="${escapeAttr(reasonText)}"` : '';

function reasonCellAttrs(extraClass) {
if (!reasonText) {
return { clickAttrs: extraClass ? ` class="${extraClass}"` : '', panel: '' };
}
const dropdownId = `reasonDropdown_${dropdownCounter++}`;
const classAttr = `clickable-state-cell${extraClass ? ' ' + extraClass : ''}`;
// tabindex + onkeydown make this reachable and operable by keyboard, not just tap -
// a bare onclick on a <td> (the existing pattern used for time/rate cell dropdowns)
// is mouse/touch-only, since <td> isn't focusable by default.
const keydown = `if(event.key==='Enter'||event.key===' '){event.preventDefault();toggleForceDropdown('${dropdownId}')}`;
return {
clickAttrs: ` onclick="toggleForceDropdown('${dropdownId}')" onkeydown="${keydown}" tabindex="0" role="button" aria-label="Why this slot" class="${classAttr}"`,
panel: `<div class="dropdown"><div id="${dropdownId}" class="dropdown-content"><div class="reason-text">${escapeAttr(reasonText)}</div></div></div>`,
};
}

let html = `<td${colspanAttr}${rowspanAttr} ${cellStyle} bgcolor=${bgColor} class="${overrideClass}"${titleAttr}>`;
const first = reasonCellAttrs(overrideClass);
let html = `<td${colspanAttr}${rowspanAttr} ${cellStyle} bgcolor=${bgColor}${first.clickAttrs}${titleAttr}>`;
html += row.state_text || '';
html += first.panel;
html += '</td>';

// Second state cell if split - same combined reason text as the first half, since
// row.reasons is a single list covering both halves of a split (e.g. charging and
// freeze-exporting in the same slot), not two separately-attributed sentences.
if (row.split && row.state2_text) {
html += `<td${rowspanAttr} ${cellStyle} bgcolor=${row.state2_color || '#FFFFFF'}${titleAttr}>${row.state2_text}</td>`;
const second = reasonCellAttrs('');
html += `<td${rowspanAttr} ${cellStyle} bgcolor=${row.state2_color || '#FFFFFF'}${second.clickAttrs}${titleAttr}>${row.state2_text}${second.panel}</td>`;
}

return html;
Expand Down
Loading