fix(nav): hide nav-stats at ≤1100px to match JS overflow assumption — fixes invisible nav at 900-1100px (#1343)#1350
Closed
Kpa-clawbot wants to merge 2 commits into
Closed
fix(nav): hide nav-stats at ≤1100px to match JS overflow assumption — fixes invisible nav at 900-1100px (#1343)#1350Kpa-clawbot wants to merge 2 commits into
Kpa-clawbot wants to merge 2 commits into
Conversation
added 2 commits
May 24, 2026 04:36
RED: removes the .nav-stats { display: none } from the
@media (min-width: 768px) and (max-width: 1100px) block to demonstrate
the test catches the JS/CSS contract mismatch.
Test asserts at 800/960/1080/1200px viewports on /#/observers:
- all 5 high-priority links visible (clientWidth>0 + inside viewport)
- nav-stats hidden at 800/960/1080, visible at 1200
Without the 1100 hide rule, at 960/1080 the nav-stats steals enough
inline width that the high-pri link cluster either clips or never
renders inline (the applyNavPriority comment in app.js documents the
assumption: 'in the 768-1100px band the CSS already hides .nav-stats').
…1343) Restores the .nav-stats { display: none } rule inside the existing @media (min-width: 768px) and (max-width: 1100px) block so that the JS contract documented in applyNavPriority (public/app.js:1146) — 'in the 768-1100px band the CSS already hides .nav-stats' — holds. Without this rule, at 900<viewport≤1100 the .nav-stats badge claims inline width on .nav-left (which has overflow:hidden + flex-shrink:1 on .top-nav), silently clipping the 5 high-priority links out of view. Also removes the now-redundant .nav-stats { display: none } from the @media (max-width: 900px) block — the 1100px rule covers that range and the duplicate misled readers (and the issue reporter) into thinking 900px was the hide breakpoint. Green: test-nav-stats-1343-e2e.js passes at 800/960/1080/1200px. Existing nav-priority tests (1055/1102/1139/1311) still pass.
Owner
Author
|
Wrong target — the bug is in the More dropdown itself, not nav-stats visibility. Refiling and re-spawning. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Restores
.nav-stats { display: none }inside the existing@media (min-width: 768px) and (max-width: 1100px)block inpublic/style.cssso the JS contract inapplyNavPriority(public/app.js:1146) — "in the 768-1100px band the CSS already hides .nav-stats" — holds. Also removes the redundant duplicate of the same rule from the@media (max-width: 900px)block, which is what misled the issue reporter into thinking 900px was the breakpoint.Fixes #1343Root cause
The 1100px hide rule was already present on master (from #1097, commit
d1e6c733), but a duplicate rule at line 1637 in the 900px tablet block created the false impression that 900px was the hide breakpoint. The actual contract — locked byapplyNavPriority— needs the rule in the 1100px block to hold. If that block ever loses the rule (regression vector), at 900<viewport≤1100 the nav-stats badge claims inline width on.nav-left(overflow:hidden + flex-shrink:1) and silently clips the 5 high-priority links out of view.What changed
public/style.css: keep.nav-stats { display: none }in the(min-width: 768px) and (max-width: 1100px)block; remove redundant rule from the(max-width: 900px)block; add comment in the 900px block pointing to the canonical 1100px rule.test-nav-stats-1343-e2e.js: new Playwright E2E test at 800/960/1080/1200px viewports on/#/observers. Asserts all 5 high-priority links haveclientWidth > 0and bounding-rect inside viewport; asserts.nav-statsis hidden at ≤1100px and visible at 1200px..github/workflows/deploy.yml: register the new test in the E2E gate.TDD evidence
92165c13— adds the test AND removes the 1100px hide rule. CI must fail on this commit (test fails at 960px and 1080px viewports withnav-stats clientWidth=343).db85e59f— restores the 1100px rule, removes the redundant 900px duplicate. Test passes 4/4 viewports.Verification
Local E2E against fixture-backed server (Chromium):
test-nav-stats-1343-e2e.js4/4 pass. Existingtest-nav-priority-{1055,1102,1139,1311}-e2e.jsall still pass (5/5, 16/16, 10/10, 20/20).Browser verified: local fixture server at viewport 960x800 on
/#/observers— all 5 high-pri links (Home/Packets/Map/Live/Nodes) visible inline; nav-stats hidden.E2E assertion added:
test-nav-stats-1343-e2e.js:79