the advice expanded inline in the table cell, running down the column and off the bottom of the screen
the labels duplicated — BREWING ADVICEADVICE, ABVABV, COMMENTNOTE, Read the adviceAdvice
The page had the new markup. Firefox was serving a stale stylesheet.
The CSS said Cache-Control: no-cache, which means revalidate — and Firefox was reusing it without revalidating. So the page was new and the stylesheet was old, and the old stylesheet had no .advice-pop rule, so the panel rendered as an ordinary <div> in the cell.
Two fixes
1. The static files now send Cache-Control: no-store.
no-cache means "revalidate before using". Firefox was not doing that. no-store is the only one that means "do not keep this". The HTML pages already sent no-store (that was #19).
2. The pop-out now carries the hidden attribute, and the script removes it when the panel opens and puts it back when it closes.
This makes the markup fail-safe rather than the stylesheet. A stylesheet that knows nothing about .advice-pop still leaves the panel hidden, so the advice can never appear expanded inside the cell.
Verified against a stale stylesheet
I removed all eight .advice-pop rules from the live stylesheet and re-measured:
with the advice-pop CSS removed:
{"removedRules": 8, "hidden": true, "display": "none",
"renderedSize": "0x0", "adviceVisible": false}
Before this change that would have rendered the advice inline in the cell — exactly your screenshot.
Still working
Tap opens the panel on a phone: hidden removed, display: block, 359×294, centred
Close hides it again: hidden back, display: none
Hover opens it on a desktop, position: fixed, 450×237
Desktop headings unchanged: RUN / NAME / GRAVITY / EST. ABV / COMMENT / BREWING ADVICE
195 tests pass, including a new one asserting the pop-out carries the hidden attribute.
On your phone
This stops it happening again. The copy Firefox already has may still be served once — a hard refresh (in Firefox for Android, tap the reload button in the menu, or long-press it) will pull the current page and stylesheet.
## The problem
Your Firefox screenshot showed two things:
- the advice **expanded inline in the table cell**, running down the column and off the bottom of the screen
- the labels **duplicated** — `BREWING ADVICEADVICE`, `ABVABV`, `COMMENTNOTE`, `Read the adviceAdvice`
The page had the new markup. Firefox was serving a **stale stylesheet**.
The CSS said `Cache-Control: no-cache`, which means *revalidate* — and Firefox was reusing it without revalidating. So the page was new and the stylesheet was old, and the old stylesheet had no `.advice-pop` rule, so the panel rendered as an ordinary `<div>` in the cell.
## Two fixes
**1. The static files now send `Cache-Control: no-store`.**
`no-cache` means "revalidate before using". Firefox was not doing that. `no-store` is the only one that means "do not keep this". The HTML pages already sent `no-store` (that was #19).
**2. The pop-out now carries the `hidden` attribute**, and the script removes it when the panel opens and puts it back when it closes.
This makes the **markup** fail-safe rather than the stylesheet. A stylesheet that knows nothing about `.advice-pop` still leaves the panel hidden, so the advice can never appear expanded inside the cell.
## Verified against a stale stylesheet
I removed all eight `.advice-pop` rules from the live stylesheet and re-measured:
```
with the advice-pop CSS removed:
{"removedRules": 8, "hidden": true, "display": "none",
"renderedSize": "0x0", "adviceVisible": false}
```
Before this change that would have rendered the advice inline in the cell — exactly your screenshot.
## Still working
- **Tap** opens the panel on a phone: `hidden` removed, `display: block`, 359×294, centred
- **Close** hides it again: `hidden` back, `display: none`
- **Hover** opens it on a desktop, `position: fixed`, 450×237
- **Desktop headings** unchanged: `RUN / NAME / GRAVITY / EST. ABV / COMMENT / BREWING ADVICE`
195 tests pass, including a new one asserting the pop-out carries the `hidden` attribute.
## On your phone
This stops it happening again. The copy Firefox already has may still be served once — a hard refresh (in Firefox for Android, tap the reload button in the menu, or long-press it) will pull the current page and stylesheet.
The screenshot from Firefox showed the advice expanded inline in the
table cell, running down the column and off the screen, with the long
and short labels both showing ("BREWING ADVICEADVICE"). The page had
the new markup and Firefox was serving a stale stylesheet.
Two fixes.
The static files now send Cache-Control: no-store. They sent
no-cache, which means "revalidate" - and Firefox was reusing them
without revalidating. no-store is the only one that means "do not
keep this". The HTML pages already sent no-store.
The pop-out carries the hidden attribute, and the script removes it
when the panel opens and puts it back when it closes. That makes the
MARKUP fail-safe rather than the stylesheet: a stylesheet that does
not know about .advice-pop at all still leaves the panel hidden, so
the advice can never appear expanded inside the cell.
Verified by removing all eight .advice-pop rules from the live
stylesheet - the panel stays hidden, 0x0, and the advice is not
rendered. Before this it would have rendered inline.
The button still opens it on tap and on hover, and the close button
still closes it. The desktop is unchanged: the same full headings and
the same hover pop-out.
195 tests pass.
PR Title: fix(ui): make the advice pop-out fail-safe against a stale stylesheet Status:⚠️ Significant Changes Needed (Pending Infrastructure/Performance Fix)
1. Executive Summary
The primary goal of this PR is to resolve a critical UI bug where stale CSS caching in browsers (specifically Firefox) causes a pop-out element to render incorrectly, breaking the table layout.
The developer has implemented a defense-in-depth strategy consisting of two parts:
UI Fail-safe: Implementing the HTML hidden attribute and JS toggles to ensure the element is invisible by default regardless of CSS state.
Cache Management: Changing the Cache-Control header from no-cache to no-store to force browsers to discard stylesheets.
The Verdict: While the UI fail-safe logic is highly praised across all reviews as a best practice, the infrastructure approach to cache management (no-store) is considered an anti-pattern that introduces significant performance risks. The PR is currently blocked by the need for a more sustainable asset-versioning strategy.
2. Prioritized List of Issues
Priority
Issue
Domain
Description
🔴Critical
Performance Degradation (no-store)
Infra / Perf
Using no-store disables all caching for static assets, forcing every single page load to perform a full round-trip to the origin server. This increases latency and server load.
🟡Medium
Lack of Asset Versioning
Infra / Code
The root cause (stale assets) is being treated with a "sledgehammer" rather than implementing industry-standard cache busting/fingerprinting.
🔵Low
Accessibility (A11y)
Code / UI
The PR does not explicitly address focus management when the hidden attribute is removed and the pop-out becomes visible.
🔵Low
CSS Override Risk
Code
Risk of future developers overriding the [hidden] attribute with a generic display: block in the CSS.
3. Domain-Specific Recommendations
🛠 Infrastructure & Performance
Immediate Action: Revert the Cache-Control: no-store change. This is unacceptable for production static assets.
Primary Recommendation: Implement Cache Busting/Fingerprinting. Append a version hash or timestamp to the CSS/JS URLs (e.g., app.css?v=1.0.2).
Optimization: Once versioning is in place, restore a performant caching strategy (e.g., max-age=31536000 for hashed files).
Architecture: Ensure that in production, /static assets are served by Nginx or a CDN rather than the Flask application process.
💻 Code Quality & UI
Keep the hidden Attribute: All reviewers agree this is a robust, semantic solution for graceful degradation.
CSS Guardrail: Add [hidden] { display: none !important; } to the global stylesheet to ensure the fail-safe cannot be accidentally overridden.
Accessibility: Review the JavaScript trigger to ensure that screen-reader focus is managed correctly when the pop-out is toggled.
🛡 Security
CSP Audit: Ensure the Content Security Policy (CSP) is configured to allow the new JS DOM manipulations without relying on 'unsafe-inline'.
Validation: Confirm that the content within the pop-out is non-sensitive, as the hidden attribute only hides elements visually and does not remove them from the DOM.
4. Summary of Positive Aspects
Defensive Programming: The move to the HTML hidden attribute is an excellent example of "Graceful Degradation," ensuring the app remains usable even if the styling layer fails.
Professional Testing: The addition of a regression test to verify the presence of the hidden attribute demonstrates high engineering standards.
Root Cause Analysis: The developer correctly identified the browser-specific caching behavior causing the UI failure.
5. Overall Assessment & Final Recommendation
Final Rating: ⚠️ SIGNIFICANT CHANGES NEEDED
The UI logic is an "Approved" addition. However, the infrastructure change is a regression in terms of performance and scalability. The PR cannot be merged in its current state because it trades system stability and user experience (latency) for a quick fix to a caching bug.
Required Action Items for Approval:
RemoveCache-Control: no-store.
Implement a cache-busting mechanism (versioning/hashing) for static assets.
Retain the hidden attribute and associated JS logic.
Add the [hidden] { display: none !important; } CSS rule.
## PR Review Results
# Consolidated Technical Review Report
**PR Title:** fix(ui): make the advice pop-out fail-safe against a stale stylesheet
**Status:** ⚠️ Significant Changes Needed (Pending Infrastructure/Performance Fix)
---
## 1. Executive Summary
The primary goal of this PR is to resolve a critical UI bug where stale CSS caching in browsers (specifically Firefox) causes a pop-out element to render incorrectly, breaking the table layout.
The developer has implemented a **defense-in-depth** strategy consisting of two parts:
1. **UI Fail-safe:** Implementing the HTML `hidden` attribute and JS toggles to ensure the element is invisible by default regardless of CSS state.
2. **Cache Management:** Changing the `Cache-Control` header from `no-cache` to `no-store` to force browsers to discard stylesheets.
**The Verdict:** While the UI fail-safe logic is highly praised across all reviews as a best practice, the infrastructure approach to cache management (`no-store`) is considered an anti-pattern that introduces significant performance risks. The PR is currently blocked by the need for a more sustainable asset-versioning strategy.
---
## 2. Prioritized List of Issues
| Priority | Issue | Domain | Description |
| :--- | :--- | :--- | :--- |
| 🔴 **Critical** | Performance Degradation (`no-store`) | Infra / Perf | Using `no-store` disables all caching for static assets, forcing every single page load to perform a full round-trip to the origin server. This increases latency and server load. |
| 🟡 **Medium** | Lack of Asset Versioning | Infra / Code | The root cause (stale assets) is being treated with a "sledgehammer" rather than implementing industry-standard cache busting/fingerprinting. |
| 🔵 **Low** | Accessibility (A11y) | Code / UI | The PR does not explicitly address focus management when the `hidden` attribute is removed and the pop-out becomes visible. |
| 🔵 **Low** | CSS Override Risk | Code | Risk of future developers overriding the `[hidden]` attribute with a generic `display: block` in the CSS. |
---
## 3. Domain-Specific Recommendations
### 🛠 Infrastructure & Performance
* **Immediate Action:** Revert the `Cache-Control: no-store` change. This is unacceptable for production static assets.
* **Primary Recommendation:** Implement **Cache Busting/Fingerprinting**. Append a version hash or timestamp to the CSS/JS URLs (e.g., `app.css?v=1.0.2`).
* **Optimization:** Once versioning is in place, restore a performant caching strategy (e.g., `max-age=31536000` for hashed files).
* **Architecture:** Ensure that in production, `/static` assets are served by Nginx or a CDN rather than the Flask application process.
### 💻 Code Quality & UI
* **Keep the `hidden` Attribute:** All reviewers agree this is a robust, semantic solution for graceful degradation.
* **CSS Guardrail:** Add `[hidden] { display: none !important; }` to the global stylesheet to ensure the fail-safe cannot be accidentally overridden.
* **Accessibility:** Review the JavaScript trigger to ensure that screen-reader focus is managed correctly when the pop-out is toggled.
### 🛡 Security
* **CSP Audit:** Ensure the Content Security Policy (CSP) is configured to allow the new JS DOM manipulations without relying on `'unsafe-inline'`.
* **Validation:** Confirm that the content within the pop-out is non-sensitive, as the `hidden` attribute only hides elements visually and does not remove them from the DOM.
---
## 4. Summary of Positive Aspects
* **Defensive Programming:** The move to the HTML `hidden` attribute is an excellent example of "Graceful Degradation," ensuring the app remains usable even if the styling layer fails.
* **Professional Testing:** The addition of a regression test to verify the presence of the `hidden` attribute demonstrates high engineering standards.
* **Root Cause Analysis:** The developer correctly identified the browser-specific caching behavior causing the UI failure.
---
## 5. Overall Assessment & Final Recommendation
**Final Rating: ⚠️ SIGNIFICANT CHANGES NEEDED**
The UI logic is an "Approved" addition. However, the infrastructure change is a regression in terms of performance and scalability. The PR cannot be merged in its current state because it trades system stability and user experience (latency) for a quick fix to a caching bug.
**Required Action Items for Approval:**
1. [ ] **Remove** `Cache-Control: no-store`.
2. [ ] **Implement** a cache-busting mechanism (versioning/hashing) for static assets.
3. [ ] **Retain** the `hidden` attribute and associated JS logic.
4. [ ] **Add** the `[hidden] { display: none !important; }` CSS rule.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
The problem
Your Firefox screenshot showed two things:
BREWING ADVICEADVICE,ABVABV,COMMENTNOTE,Read the adviceAdviceThe page had the new markup. Firefox was serving a stale stylesheet.
The CSS said
Cache-Control: no-cache, which means revalidate — and Firefox was reusing it without revalidating. So the page was new and the stylesheet was old, and the old stylesheet had no.advice-poprule, so the panel rendered as an ordinary<div>in the cell.Two fixes
1. The static files now send
Cache-Control: no-store.no-cachemeans "revalidate before using". Firefox was not doing that.no-storeis the only one that means "do not keep this". The HTML pages already sentno-store(that was #19).2. The pop-out now carries the
hiddenattribute, and the script removes it when the panel opens and puts it back when it closes.This makes the markup fail-safe rather than the stylesheet. A stylesheet that knows nothing about
.advice-popstill leaves the panel hidden, so the advice can never appear expanded inside the cell.Verified against a stale stylesheet
I removed all eight
.advice-poprules from the live stylesheet and re-measured:Before this change that would have rendered the advice inline in the cell — exactly your screenshot.
Still working
hiddenremoved,display: block, 359×294, centredhiddenback,display: noneposition: fixed, 450×237RUN / NAME / GRAVITY / EST. ABV / COMMENT / BREWING ADVICE195 tests pass, including a new one asserting the pop-out carries the
hiddenattribute.On your phone
This stops it happening again. The copy Firefox already has may still be served once — a hard refresh (in Firefox for Android, tap the reload button in the menu, or long-press it) will pull the current page and stylesheet.
The screenshot from Firefox showed the advice expanded inline in the table cell, running down the column and off the screen, with the long and short labels both showing ("BREWING ADVICEADVICE"). The page had the new markup and Firefox was serving a stale stylesheet. Two fixes. The static files now send Cache-Control: no-store. They sent no-cache, which means "revalidate" - and Firefox was reusing them without revalidating. no-store is the only one that means "do not keep this". The HTML pages already sent no-store. The pop-out carries the hidden attribute, and the script removes it when the panel opens and puts it back when it closes. That makes the MARKUP fail-safe rather than the stylesheet: a stylesheet that does not know about .advice-pop at all still leaves the panel hidden, so the advice can never appear expanded inside the cell. Verified by removing all eight .advice-pop rules from the live stylesheet - the panel stays hidden, 0x0, and the advice is not rendered. Before this it would have rendered inline. The button still opens it on tap and on hover, and the close button still closes it. The desktop is unchanged: the same full headings and the same hover pop-out. 195 tests pass.PR received — starting review, sit tight 🫡
PR Review Results
Consolidated Technical Review Report
PR Title: fix(ui): make the advice pop-out fail-safe against a stale stylesheet
Status: ⚠️ Significant Changes Needed (Pending Infrastructure/Performance Fix)
1. Executive Summary
The primary goal of this PR is to resolve a critical UI bug where stale CSS caching in browsers (specifically Firefox) causes a pop-out element to render incorrectly, breaking the table layout.
The developer has implemented a defense-in-depth strategy consisting of two parts:
hiddenattribute and JS toggles to ensure the element is invisible by default regardless of CSS state.Cache-Controlheader fromno-cachetono-storeto force browsers to discard stylesheets.The Verdict: While the UI fail-safe logic is highly praised across all reviews as a best practice, the infrastructure approach to cache management (
no-store) is considered an anti-pattern that introduces significant performance risks. The PR is currently blocked by the need for a more sustainable asset-versioning strategy.2. Prioritized List of Issues
no-store)no-storedisables all caching for static assets, forcing every single page load to perform a full round-trip to the origin server. This increases latency and server load.hiddenattribute is removed and the pop-out becomes visible.[hidden]attribute with a genericdisplay: blockin the CSS.3. Domain-Specific Recommendations
🛠 Infrastructure & Performance
Cache-Control: no-storechange. This is unacceptable for production static assets.app.css?v=1.0.2).max-age=31536000for hashed files)./staticassets are served by Nginx or a CDN rather than the Flask application process.💻 Code Quality & UI
hiddenAttribute: All reviewers agree this is a robust, semantic solution for graceful degradation.[hidden] { display: none !important; }to the global stylesheet to ensure the fail-safe cannot be accidentally overridden.🛡 Security
'unsafe-inline'.hiddenattribute only hides elements visually and does not remove them from the DOM.4. Summary of Positive Aspects
hiddenattribute is an excellent example of "Graceful Degradation," ensuring the app remains usable even if the styling layer fails.hiddenattribute demonstrates high engineering standards.5. Overall Assessment & Final Recommendation
Final Rating: ⚠️ SIGNIFICANT CHANGES NEEDED
The UI logic is an "Approved" addition. However, the infrastructure change is a regression in terms of performance and scalability. The PR cannot be merged in its current state because it trades system stability and user experience (latency) for a quick fix to a caching bug.
Required Action Items for Approval:
Cache-Control: no-store.hiddenattribute and associated JS logic.[hidden] { display: none !important; }CSS rule.