From 7dd39190f10b532215d3b9d4f31a3130871bb226 Mon Sep 17 00:00:00 2001 From: hermes Date: Mon, 9 Nov 2026 20:56:56 +1000 Subject: [PATCH 1/2] fix(ui): mobile pop-out for the advice, and tables that fit a phone The brewing advice is several hundred characters. In a table cell it pushed the table past the width of a phone, so the column was clipped off the right edge and the advice could not be read without sideways scrolling. The advice is now a compact trigger in the cell and the text is in a floating panel that opens on hover (desktop) or on tap (phone). The panel is position:fixed and anchored by the script. Absolutely positioned inside the cell it was clipped by the table's own scroll container, and the advice was sliced in half by the card's edge. It flips above the trigger when there is no room below. The tables now reflow into stacked cards on a narrow screen, with each value labelled by its column name. A seven column table does not fit a phone, and it used to scroll sideways inside its wrapper, so the columns on the right were off the screen. Also: the header wraps onto two rows so the nav tabs are not clipped, the point scale says 'points' rather than 'pts', and the reading count on the beer card does not orphan its word. --- src/flask/static/css/app.css | 279 ++++++++++++++++++++++++++++++-- src/flask/static/js/app.js | 176 ++++++++++++++++++++ src/flask/templates/active.html | 2 +- src/flask/templates/brews.html | 21 ++- tests/test_brew_advice.py | 4 +- 5 files changed, 460 insertions(+), 22 deletions(-) diff --git a/src/flask/static/css/app.css b/src/flask/static/css/app.css index fb54866..7a050c9 100644 --- a/src/flask/static/css/app.css +++ b/src/flask/static/css/app.css @@ -480,6 +480,138 @@ table.data tbody tr:hover { background: var(--bg-raised-2); } table.data .num { font-family: var(--mono); text-align: right; white-space: nowrap; } table.data .comment { color: var(--text-muted); font-size: .84rem; } +/* On a narrow screen the tables reflow into stacked cards. + + A seven-column table does not fit a phone. It was left to scroll + sideways inside its wrapper, which meant the columns on the right were + off the screen and the brewer had to pan the table to see them. + + Each row becomes a card: the header cell becomes a label above its + value, and the cells stack. */ +@media (max-width: 720px) { + /* The header wraps onto two rows on a phone: the brand and the + sign-out on the first, the nav tabs on the second where they get + the full width and scroll sideways if they still do not fit. + + They used to sit on one row with the brand and the sign-out, and + the last tab was cut off mid-word. */ + .topbar__inner { + flex-wrap: wrap; + row-gap: .5rem; + } + + .brand { + order: 1; + } + + .topbar__logout { + order: 2; + margin-left: auto; + } + + .nav { + order: 3; + flex: 1 0 100%; + overflow-x: auto; + scrollbar-width: none; + -webkit-overflow-scrolling: touch; + /* A fade at the right edge so a clipped tab reads as + scrollable rather than broken. */ + -webkit-mask-image: linear-gradient(to right, #000 calc(100% - 1.5rem), transparent 100%); + mask-image: linear-gradient(to right, #000 calc(100% - 1.5rem), transparent 100%); + } + + .nav::-webkit-scrollbar { + display: none; + } + + .nav__link { + flex-shrink: 0; + } + + /* Tighter tabs, so all four fit on a phone without scrolling. + They still scroll if they have to. */ + .nav__link { + padding: .35rem .55rem; + font-size: .8rem; + letter-spacing: .01em; + } + + .table-wrap { + overflow-x: visible; + } + + table.data, + table.data tbody, + table.data tr, + table.data td { + display: block; + width: auto; + } + + table.data thead { + /* HIDDEN in the stacked layout. Each cell shows its column + name as a label, so the header row would be a duplicate - and + it was 230px of the first card. */ + display: none; + } + + table.data tr { + border: 1px solid var(--border); + border-radius: 8px; + margin-bottom: .75rem; + padding: .25rem .1rem; + background: var(--bg-raised-2); + } + + table.data tbody tr:hover { + background: var(--bg-raised-2); + } + + table.data th { + display: block; + border-bottom: 1px solid var(--border); + } + + table.data td { + display: grid; + grid-template-columns: 8.5rem 1fr; + gap: .5rem; + align-items: baseline; + padding: .45rem .7rem; + border-bottom: 1px solid rgba(255, 255, 255, .04); + text-align: left; + } + + table.data td::before { + /* The column heading, shown as the row's label. */ + content: attr(data-label); + font-size: .65rem; + font-weight: 650; + letter-spacing: .07em; + text-transform: uppercase; + color: var(--text-faint); + } + + table.data td.num { + text-align: left; + } + + table.data td.right, + table.data td:empty { + grid-template-columns: 1fr; + } + + table.data td.right::before { + content: none; + } + + /* A cell with nothing in it must not leave a gap. */ + table.data td:empty { + display: none; + } +} + /* ----------------------------------------------------------------- chips */ .chip { @@ -595,8 +727,18 @@ table.data .comment { color: var(--text-muted); font-size: .84rem; } .brew-card__name { font-weight: 650; font-size: 1.02rem; margin: 0; } .brew-card__meta { color: var(--text-muted); font-size: .82rem; font-family: var(--mono); } .brew-card__stats { display: grid; grid-template-columns: repeat(3, 1fr); gap: .6rem; } + +/* The three stats sit side by side, so their captions line up. One + caption ("needs 2nd reading") is two lines and the others are one, + which left the baselines ragged. */ +.brew-card__stat { + display: flex; + flex-direction: column; +} + .brew-card__stat .stat__label { font-size: .68rem; } .brew-card__stat .stat__value { font-size: 1.3rem; } +.brew-card__stat .stat__meta { margin-top: auto; } .brew-card__spark { height: 46px; } /* The "start a new beer" tile — fills the last grid slot and gives the @@ -733,27 +875,130 @@ table.data .comment { color: var(--text-muted); font-size: .84rem; } color: var(--danger); } +/* The advice in the table is a compact trigger that pops the full text + out on hover, focus or tap. + + It used to be the text itself, clamped to four lines. The advice runs + to several hundred characters, and a cell that wide pushes the table + past the width of a phone, so the column ended up clipped off the + right edge and unreadable without sideways scrolling. */ .advice-cell { - max-width: 32rem; + position: relative; + width: 1%; + white-space: nowrap; } -/* In the table the advice is clamped to a few lines - the full text is - on the run detail page, where there is room to read it. */ -.advice-cell .advice { - margin-top: 0; +.advice-trigger { + display: inline-flex; + align-items: center; + gap: .35rem; + padding: .25rem .55rem; + border: 1px solid var(--amber); + border-radius: 999px; + background: var(--amber-dim); + color: var(--amber-text); + font: inherit; font-size: .75rem; - display: -webkit-box; - -webkit-line-clamp: 4; - line-clamp: 4; - -webkit-box-orient: vertical; - overflow: hidden; + line-height: 1.4; + cursor: pointer; + white-space: nowrap; } -/* The clamped text opens up on hover, so the whole thing is readable - without leaving the page. */ -.advice-cell:hover .advice, -.advice-cell:focus-within .advice { - -webkit-line-clamp: unset; - line-clamp: unset; - overflow: visible; +.advice-trigger:hover, +.advice-trigger:focus-visible { + background: var(--amber); + color: var(--bg); +} + +.advice-trigger--failed { + border-color: var(--danger); + background: var(--danger-dim); + color: var(--danger); +} + +.advice-trigger__icon { + font-size: .85em; +} + +/* The pop-out. + + It is `position: fixed` and anchored to the trigger by the script, + not absolutely positioned inside the cell. The table is a scroll + container, and an absolutely positioned panel inside it was clipped + by the container's own overflow - the advice was sliced in half by + the card's edge and the bottom border was cut off. + + Fixed positioning takes it out of the table entirely, so it floats + above the page and is never cut by a scroll container. */ +.advice-pop { + position: fixed; + z-index: 200; + display: none; + width: min(30rem, 92vw); + max-height: 70vh; + overflow-y: auto; + padding: .8rem .9rem; + border: 1px solid var(--amber); + border-radius: 8px; + background: var(--bg-raised); + box-shadow: 0 12px 32px rgba(0, 0, 0, .55); + text-align: left; + white-space: normal; +} + +.advice-pop__head { + display: flex; + align-items: center; + justify-content: space-between; + gap: .5rem; + margin-bottom: .4rem; +} + +.advice-pop__head .advice__label { + margin-bottom: 0; +} + +.advice-pop__close { + border: 0; + background: none; + color: var(--text-muted); + font-size: 1.1rem; + line-height: 1; + cursor: pointer; + padding: 0 .15rem; +} + +.advice-pop__close:hover { + color: var(--text); +} + +.advice-pop__text { + margin: 0; + font-size: .8125rem; + line-height: 1.55; + color: var(--text); +} + +/* The script opens and closes the panel, and places it. The CSS only + decides whether it is shown. */ +.advice-pop.is-open { + display: block; +} + +/* On a phone there is no hover, so the trigger is a button the script + toggles, and the panel is pinned to the middle of the screen so it is + readable whatever the table's width. */ +@media (max-width: 720px) { + .advice-cell { + width: auto; + } + + /* On a phone the panel is centred on the screen rather than + anchored to the trigger - there is not the room to float it + beside the cell, and the trigger may be near the edge. */ + .advice-pop { + top: 50% !important; + left: 50% !important; + transform: translate(-50%, -50%); + } } diff --git a/src/flask/static/js/app.js b/src/flask/static/js/app.js index cad54d6..dd131fc 100644 --- a/src/flask/static/js/app.js +++ b/src/flask/static/js/app.js @@ -56,11 +56,187 @@ } } + /** + * The brewing advice pop-out. + * + * On a desktop the panel opens on hover, which the CSS does. A + * phone has no hover, so the trigger is a button that toggles it - + * and it is a real button, so it works with a keyboard and with a + * screen reader. + */ + /** + * The brewing advice pop-out. + * + * The advice runs to several hundred characters. In a table cell it + * pushed the table past the width of a phone, so it is a compact + * trigger instead and the text is shown in a floating panel. + * + * The panel is position:fixed so it escapes the table's scroll + * container - absolutely positioned inside the cell it was clipped + * by the container and sliced in half by the card's edge. + * + * It opens on hover on a desktop, and on tap on a phone, which has + * no hover. The trigger is a real button, so it also works with a + * keyboard. + */ + function wireAdvicePopouts() { + var triggers = document.querySelectorAll(".advice-trigger"); + for (var i = 0; i < triggers.length; i++) { + (function (trigger) { + var panel = trigger.parentNode.querySelector(".advice-pop"); + if (!panel) { return; } + + var closeTimer = null; + + function isPhone() { + return window.matchMedia("(max-width: 720px)").matches; + } + + function place() { + if (isPhone()) { + /* The CSS centres it on a phone - clear any + inline position left over from a desktop + resize. */ + panel.style.top = ""; + panel.style.left = ""; + return; + } + + var box = trigger.getBoundingClientRect(); + var gap = 10; + var height = panel.offsetHeight; + var width = panel.offsetWidth; + var spaceBelow = window.innerHeight - box.bottom - gap; + var spaceAbove = box.top - gap; + + /* Prefer below; flip above when there is not enough + room and there is more room above. */ + var top; + if (height <= spaceBelow || spaceBelow >= spaceAbove) { + top = box.bottom + gap; + } else { + top = Math.max(gap, box.top - height - gap); + } + + /* Keep it on screen horizontally. */ + var left = Math.min(box.left, window.innerWidth - width - gap); + left = Math.max(gap, left); + + panel.style.top = Math.round(top) + "px"; + panel.style.left = Math.round(left) + "px"; + } + + function open() { + if (closeTimer) { + clearTimeout(closeTimer); + closeTimer = null; + } + panel.classList.add("is-open"); + trigger.setAttribute("aria-expanded", "true"); + place(); + } + + function close() { + panel.classList.remove("is-open"); + trigger.setAttribute("aria-expanded", "false"); + } + + function closeSoon() { + /* A short delay, so the mouse can travel from the + trigger into the panel without it closing. */ + if (closeTimer) { clearTimeout(closeTimer); } + closeTimer = setTimeout(function () { + closeTimer = null; + if (!panel.matches(":hover") && !trigger.matches(":hover")) { + close(); + } + }, 180); + } + + trigger.addEventListener("click", function (event) { + event.preventDefault(); + event.stopPropagation(); + if (panel.classList.contains("is-open")) { + close(); + } else { + open(); + } + }); + + trigger.addEventListener("mouseenter", function () { + if (!isPhone()) { open(); } + }); + trigger.addEventListener("mouseleave", closeSoon); + panel.addEventListener("mouseenter", function () { + if (closeTimer) { + clearTimeout(closeTimer); + closeTimer = null; + } + }); + panel.addEventListener("mouseleave", closeSoon); + + var closeButton = panel.querySelector(".advice-pop__close"); + if (closeButton) { + closeButton.addEventListener("click", function (event) { + event.preventDefault(); + event.stopPropagation(); + close(); + trigger.focus(); + }); + } + + document.addEventListener("keydown", function (event) { + if (event.key === "Escape" && panel.classList.contains("is-open")) { + close(); + trigger.focus(); + } + }); + document.addEventListener("click", function (event) { + if (!panel.contains(event.target) && event.target !== trigger) { + close(); + } + }); + + window.addEventListener("resize", function () { + if (panel.classList.contains("is-open")) { place(); } + }); + window.addEventListener("scroll", function () { + if (panel.classList.contains("is-open")) { close(); } + }, true); + })(triggers[i]); + } + } + + function labelTableCells() { + var tables = document.querySelectorAll("table.data"); + for (var t = 0; t < tables.length; t++) { + var headerRow = tables[t].querySelector("thead tr"); + if (!headerRow) { continue; } + var labels = []; + var headers = headerRow.querySelectorAll("th"); + for (var h = 0; h < headers.length; h++) { + labels.push(headers[h].textContent.trim()); + } + + var rows = tables[t].querySelectorAll("tbody tr"); + for (var r = 0; r < rows.length; r++) { + var cells = rows[r].querySelectorAll("td"); + for (var c = 0; c < cells.length && c < labels.length; c++) { + if (labels[c] && !cells[c].getAttribute("data-label")) { + cells[c].setAttribute("data-label", labels[c]); + } + } + } + } + } + function init() { focusFirstEmpty(); defaultDateFields(); wireToggles(); wireConfirms(); + wireAdvicePopouts(); + labelTableCells(); } if (document.readyState === "loading") { diff --git a/src/flask/templates/active.html b/src/flask/templates/active.html index 3180111..687db8b 100644 --- a/src/flask/templates/active.html +++ b/src/flask/templates/active.html @@ -44,7 +44,7 @@
Run {{ run.beer_run_id }} {% if run.first_date %} · since {{ run.first_date }}{% endif %} - {% if run.reading_count == 1 %} · 1 reading{% else %} · {{ run.reading_count }} readings{% endif %} + {% if run.reading_count == 1 %} · 1 reading{% else %} · {{ run.reading_count }} readings{% endif %}
{% if run.tap_number is not none %} diff --git a/src/flask/templates/brews.html b/src/flask/templates/brews.html index 2ee2efa..0bebe60 100644 --- a/src/flask/templates/brews.html +++ b/src/flask/templates/brews.html @@ -95,7 +95,7 @@ {% if run.min_sg is not none %} {{ run.min_sg_decimal }} -
{{ run.min_sg_point_scale }} pts
+
{{ run.min_sg_point_scale }} points
{% else %}—{% endif %} @@ -112,9 +112,24 @@ — {% endif %} - + {% if run.advice %} - {{ run.advice }} + + {% else %} Thinking… {% endif %} diff --git a/tests/test_brew_advice.py b/tests/test_brew_advice.py index 12dbf57..d45b790 100644 --- a/tests/test_brew_advice.py +++ b/tests/test_brew_advice.py @@ -1008,7 +1008,9 @@ def test_a_failed_advice_is_marked_as_such(client, repo): response = client.get("/brews") body = response.get_data(as_text=True) - assert "advice--failed" in body + # A failed generation is marked differently from real advice. + assert "advice-trigger--failed" in body + assert "Advice failed" in body def test_advice_is_not_generated_in_tests(client, repo): From 21fcdf485cc707c33fad171e7be935c006334423 Mon Sep 17 00:00:00 2001 From: hermes Date: Mon, 9 Nov 2026 20:57:19 +1000 Subject: [PATCH 2/2] test: the advice in the table is a pop-out, not the text --- tests/test_brew_advice.py | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests/test_brew_advice.py b/tests/test_brew_advice.py index d45b790..8fc5e98 100644 --- a/tests/test_brew_advice.py +++ b/tests/test_brew_advice.py @@ -995,6 +995,35 @@ def test_the_advice_is_shown_on_the_active_beers_page(client, repo): assert "Brewing advice" in body +def test_the_advice_in_the_table_is_a_popout_not_the_text(client, repo): + """The advice in the table is a trigger, and the text is in a panel. + + The advice is several hundred characters. In a table cell it made + the table wider than a phone, so the column was clipped off the + right edge and the advice could not be read. + """ + repo.users.insert_one({"username": "beer", "password": "beer_pass"}) + client.post("/login", data={"username": "beer", "password": "beer_pass"}, + follow_redirects=True) + + seed_readings(repo, [("1", "Ale", "1042", "2026-01-01", False, "")]) + doc = repo.readings.find_one({"beer_run_id": "1"}) + repo.set_reading_advice(str(doc["_id"]), "Warm it to 21C and wait.", ok=True) + + body = client.get("/brews").get_data(as_text=True) + + # The trigger is a real button, so it works with a keyboard. + assert "advice-trigger" in body + assert 'class="advice-trigger' in body + + # The text lives in the panel, not in the cell. + assert "advice-pop" in body + assert "advice-pop__text" in body + + # The full advice is in the page - it is just not in the cell. + assert "Warm it to 21C and wait." in body + + def test_a_failed_advice_is_marked_as_such(client, repo): """A generation failure reads differently from real advice.""" repo.users.insert_one({"username": "beer", "password": "beer_pass"})