forgejo_admin/pal-e-api

Fork 1

fix: nav wrap bug + mobile spacing polish (#49) #50

Merged

forgejo_admin merged 2 commits from 49-visual-qa-polish into main

2026-02-27 18:43:05 +00:00

forgejo_admin commented

2026-02-27 18:05:00 +00:00

Owner

Summary

Fix mobile nav wrap bug: brand, nav-links, and nav-auth now each stack on their own row at 375px viewport
Increase mobile section spacing from 2.5rem to 3rem for better vertical breathing room
Add playwright test validating 3-row nav layout

Changes

src/pal_e_docs/templates/base.html: flex-basis: 100% → flex: 0 0 100% for .brand, .nav-links, .nav-auth in mobile breakpoint; added .section { margin-bottom: 3rem } in mobile breakpoint
tests/test_mobile_responsive.py: added test #6 test_nav_three_row_layout — verifies brand, nav-links, nav-auth have strictly increasing Y positions at 375px

Test Plan

All 152 tests pass locally (PALDOCS_DATABASE_PATH=:memory: python -m pytest)
All 9 playwright browser tests pass (including new test_nav_three_row_layout)
No regressions in nav overflow, table scroll, or font size tests

Review Checklist

Passed automated review-fix loop
No secrets committed
No unnecessary file changes
Commit messages are descriptive

plan-2026-02-27-responsive-design-mobile-ux — Phase 3: Visual QA + polish
Forgejo issue #49

## Summary - Fix mobile nav wrap bug: brand, nav-links, and nav-auth now each stack on their own row at 375px viewport - Increase mobile section spacing from 2.5rem to 3rem for better vertical breathing room - Add playwright test validating 3-row nav layout ## Changes - `src/pal_e_docs/templates/base.html`: `flex-basis: 100%` → `flex: 0 0 100%` for `.brand`, `.nav-links`, `.nav-auth` in mobile breakpoint; added `.section { margin-bottom: 3rem }` in mobile breakpoint - `tests/test_mobile_responsive.py`: added test #6 `test_nav_three_row_layout` — verifies brand, nav-links, nav-auth have strictly increasing Y positions at 375px ## Test Plan - [x] All 152 tests pass locally (`PALDOCS_DATABASE_PATH=:memory: python -m pytest`) - [x] All 9 playwright browser tests pass (including new `test_nav_three_row_layout`) - [x] No regressions in nav overflow, table scroll, or font size tests ## Review Checklist - [ ] Passed automated review-fix loop - [x] No secrets committed - [x] No unnecessary file changes - [x] Commit messages are descriptive ## Related Notes - `plan-2026-02-27-responsive-design-mobile-ux` — Phase 3: Visual QA + polish - Forgejo issue #49

forgejo_admin added 1 commit

2026-02-27 18:05:00 +00:00

fix: nav wrap bug at 375px + mobile section spacing (#49 )

ci/woodpecker/pr/woodpecker Pipeline failed

Details

7454567bbb

The nav's flex children used `flex-basis: 100%` which allowed
flex-shrink to keep items on the same line. Changed to
`flex: 0 0 100%` (grow:0 shrink:0 basis:100%) so brand,
nav-links, and nav-auth each force their own row on mobile.

Also increased `.section` margin-bottom from 2.5rem to 3rem
on mobile for better vertical breathing room.

Added playwright test (test #6) verifying 3-row nav layout
by checking that brand, nav-links, and nav-auth have strictly
increasing Y positions at 375px viewport.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

forgejo_admin commented

2026-02-27 18:05:24 +00:00

Author

Owner

Review Pass — Clean

Round 1 review: No issues found.

CSS Fix

flex: 0 0 100% correctly disables flex-shrink, forcing each nav child onto its own row. Root cause properly identified and fixed.
Mobile .section margin bump (2.5rem → 3rem) is a modest, reasonable increase.

Test

test_nav_three_row_layout follows existing test patterns and uses getBoundingClientRect().top to verify 3-row stacking — correct approach.
Good assertion messages with actual values for debugging.

Verification

152/152 tests passing, including 9 playwright browser tests.
No unnecessary file changes, no secrets.

Ready for merge.

## Review Pass — Clean **Round 1 review: No issues found.** ### CSS Fix - `flex: 0 0 100%` correctly disables flex-shrink, forcing each nav child onto its own row. Root cause properly identified and fixed. - Mobile `.section` margin bump (2.5rem → 3rem) is a modest, reasonable increase. ### Test - `test_nav_three_row_layout` follows existing test patterns and uses `getBoundingClientRect().top` to verify 3-row stacking — correct approach. - Good assertion messages with actual values for debugging. ### Verification - 152/152 tests passing, including 9 playwright browser tests. - No unnecessary file changes, no secrets. **Ready for merge.**

forgejo_admin commented

2026-02-27 18:08:18 +00:00

Author

Owner

PR Review — Round 1

Scope

2 files changed (CSS + test only). No scope creep.

CSS Fix: `flex-basis: 100%` → `flex: 0 0 100%`

Correct fix. The shorthand locks flex-grow: 0, flex-shrink: 0, flex-basis: 100%, eliminating the shrink loophole that flex-basis: 100% alone leaves (default flex-shrink: 1 could let the browser compress below 100%). Each nav child is now rigidly full-width inside the flex-wrap container — guaranteed 3 rows.

All three changes are inside @media (max-width: 600px). Desktop nav has no competing flex shorthand, so no regression risk.

Mobile Section Spacing: 2.5rem → 3rem

Correctly overrides the desktop .section { margin-bottom: 2.5rem; } base rule. Properly scoped inside the media query.

Test: `test_nav_three_row_layout`

Good test. Uses getBoundingClientRect().top to verify strictly increasing Y positions for brand, nav-links, and nav-auth. Tests actual geometry, not just DOM existence. Clear assertion messages.

Suggestions (non-blocking, can fix later)

Minimum gap assertion — links_top > brand_top passes even if the gap is 0.1px. A links_top > brand_top + 5 threshold would catch degenerate cases. Theoretical concern only — real content makes this moot.
No test for section spacing change — minor CSS tweak, acceptable to leave untested.

Verdict: ✅ APPROVE

Clean, minimal, correct. No blocking issues.

## PR Review — Round 1 ### Scope **2 files changed** (CSS + test only). No scope creep. ### CSS Fix: `flex-basis: 100%` → `flex: 0 0 100%` Correct fix. The shorthand locks `flex-grow: 0`, `flex-shrink: 0`, `flex-basis: 100%`, eliminating the shrink loophole that `flex-basis: 100%` alone leaves (default `flex-shrink: 1` could let the browser compress below 100%). Each nav child is now rigidly full-width inside the flex-wrap container — guaranteed 3 rows. All three changes are inside `@media (max-width: 600px)`. Desktop nav has no competing `flex` shorthand, so no regression risk. ### Mobile Section Spacing: 2.5rem → 3rem Correctly overrides the desktop `.section { margin-bottom: 2.5rem; }` base rule. Properly scoped inside the media query. ### Test: `test_nav_three_row_layout` Good test. Uses `getBoundingClientRect().top` to verify strictly increasing Y positions for brand, nav-links, and nav-auth. Tests actual geometry, not just DOM existence. Clear assertion messages. ### Suggestions (non-blocking, can fix later) 1. **Minimum gap assertion** — `links_top > brand_top` passes even if the gap is 0.1px. A `links_top > brand_top + 5` threshold would catch degenerate cases. Theoretical concern only — real content makes this moot. 2. **No test for section spacing change** — minor CSS tweak, acceptable to leave untested. ### Verdict: ✅ APPROVE Clean, minimal, correct. No blocking issues.

forgejo_admin added 1 commit

2026-02-27 18:34:53 +00:00

fix: strengthen nav row layout assertions with 10px minimum gap

ci/woodpecker/pr/woodpecker Pipeline failed

Details

627eed814f

The previous assertions (links_top > brand_top) were too weak — they
would pass even with sub-pixel Y differences where elements are
visually on the same row. Now requires at least 10px of vertical
separation to confirm genuinely separate visual rows.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>