-
Notifications
You must be signed in to change notification settings - Fork 0
Fix mobile menu: restore dropdown links, remove Systems, refine spacing (LS-3222) #58
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.
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
Merged
brandonmarshal
merged 7 commits into
develop
from
feature/ls-3222-fix-mobile-menu-restore-links-and-remove-systems
Sep 17, 2026
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
9e863df
Fix mobile menu: restore links, remove Systems, refine dropdown spaci…
brandonmarshal 3999cc5
Refine mobile Services dropdown to single-column layout (LS-3222)
brandonmarshal 12191b8
Make mobile accordion labels link to their overview pages (LS-3222)
brandonmarshal c3aed5d
Add changelog entry for mobile menu fix (LS-3222)
brandonmarshal 1feb6df
Address valid CodeRabbit/Copilot review findings on PR #58 (LS-3222)
brandonmarshal 7d15c3c
Shorten shared mega menu row style description
coderabbitai[bot] 131c70a
Merge develop into LS-3222 mobile menu branch
brandonmarshal File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Large diffs are not rendered by default.
Oops, something went wrong.
Large diffs are not rendered by default.
Oops, something went wrong.
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| # Specification Quality Checklist: Fix Mobile Menu — Restore Links and Remove Systems | ||
|
|
||
| **Purpose**: Validate specification completeness and quality before proceeding to planning | ||
| **Created**: 2026-09-14 | ||
| **Feature**: [spec.md](../spec.md) | ||
|
|
||
| ## Content Quality | ||
|
|
||
| - [x] No implementation details (languages, frameworks, APIs) | ||
| - [x] Focused on user value and business needs | ||
| - [x] Written for non-technical stakeholders | ||
| - [x] All mandatory sections completed | ||
|
|
||
| ## Requirement Completeness | ||
|
|
||
| - [x] No [NEEDS CLARIFICATION] markers remain | ||
| - [x] Requirements are testable and unambiguous | ||
| - [x] Success criteria are measurable | ||
| - [x] Success criteria are technology-agnostic (no implementation details) | ||
| - [x] All acceptance scenarios are defined | ||
| - [x] Edge cases are identified | ||
| - [x] Scope is clearly bounded | ||
| - [x] Dependencies and assumptions identified | ||
|
|
||
| ## Feature Readiness | ||
|
|
||
| - [x] All functional requirements have clear acceptance criteria | ||
| - [x] User scenarios cover primary flows | ||
| - [x] Feature meets measurable outcomes defined in Success Criteria | ||
| - [x] No implementation details leak into specification | ||
|
|
||
| ## Notes | ||
|
|
||
| - All checklist items pass on first pass. No clarifications required — reasonable defaults documented in the Assumptions section of spec.md. | ||
| - "No implementation details" (Content Quality and Feature Readiness) is scoped to the Requirements and Success Criteria sections, which stay technology-agnostic. The Assumptions section intentionally references real file paths (e.g. `parts/mobile-menu.html`) and technical terms (CSS/markup/JS) for grounding and traceability back to the actual codebase — consistent with this project's existing spec convention — rather than describing implementation choices as requirements. | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| # Phase 1 Data Model: Fix Mobile Menu — Restore Links and Remove Systems | ||
|
|
||
| This feature has no persisted data, database schema, or state transitions — it is a front-end | ||
| markup/CSS fix to a static WordPress template part. The "entities" below describe the structural | ||
| concepts in the markup, for traceability against the functional requirements in | ||
| [spec.md](./spec.md), not a data model in the traditional sense. | ||
|
|
||
| ## Mobile Menu | ||
|
|
||
| **Represents**: The collapsible mobile navigation surface rendered inside WordPress core's | ||
| Navigation block responsive/overlay container. | ||
|
|
||
| **Source**: `parts/mobile-menu.html` | ||
|
|
||
| **Key attributes**: | ||
| - Brand row (logo) | ||
| - A sequence of top-level entries, each either: | ||
| - An accordion (`<details class="mobile-menu-accordion is-style-mobile-menu-accordion">`) containing | ||
| a page-list dropdown, or | ||
| - A standalone link row (`mobile-menu-link-row`) | ||
| - A trailing actions block (buttons: "Start a project", "Explore case studies") | ||
|
|
||
| **Relationships**: Contains one or more Menu Items (below); wrapped by core's | ||
| `.wp-block-navigation__responsive-container` overlay when open. | ||
|
|
||
| ## Menu Item | ||
|
|
||
| **Represents**: A single navigable entry, either top-level or nested inside an accordion's page list. | ||
|
|
||
| **Variants present in `parts/mobile-menu.html`**: | ||
| - **Accordion group** — `<details>` with a `<summary>` label (Work, Solutions, Services, Pricing, | ||
| Insights, About), containing a page-list grid (`.ls-simple-submenu-links` or, for Services, phase | ||
| groups `.ls-services-phase-links`) of individual link rows styled `is-style-mega-menu-item-service`, | ||
| plus a trailing "See all …" link styled `is-style-link-arrow-accent`. | ||
| - **Standalone link row** — a bare `<p class="mobile-menu-link-row is-style-mobile-menu-link-row">` | ||
| containing one `<a>`, used for "Systems" (to be removed) and "Contact" (unaffected). | ||
|
|
||
| **Key attributes**: label text, `href` destination, containing style class (determines padding/hover | ||
| behavior/hit-area via `styles/blocks/groups/mega-menu-item-service.json` and related JSON partials). | ||
|
|
||
| **Validation rule (from FR-001/FR-002)**: every `<a>` inside the mobile menu must remain reachable and | ||
| clickable regardless of which variant/style class it uses. | ||
|
|
||
| ## Systems Item (removed) | ||
|
|
||
| **Represents**: The specific standalone link row (`href="/systems/"`, label "Systems") at | ||
| `parts/mobile-menu.html` lines 214–216, sitting between the "Services" and "Pricing" accordions. | ||
|
|
||
| **Lifecycle**: Deleted outright (see [research.md](./research.md) R2) — not hidden, not repointed. | ||
| No other menu item depends on or nests inside it, so removal has no cascading structural impact. |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,93 @@ | ||
| # Implementation Plan: Fix Mobile Menu — Restore Links and Remove Systems | ||
|
|
||
| **Branch**: `001-fix-mobile-menu` | **Date**: 2026-09-14 | **Spec**: [spec.md](./spec.md) | ||
|
|
||
| **Input**: Feature specification from `/specs/001-fix-mobile-menu/spec.md` | ||
|
|
||
| **Note**: This template is filled in by the `/speckit-plan` command; its definition describes the execution workflow. | ||
|
|
||
| ## Summary | ||
|
|
||
| Mobile menu links in `parts/mobile-menu.html` (rendered inside WordPress core's Navigation block responsive/overlay container) are not clickable, and a dead `/systems/` link row must be removed. The fix is CSS/markup-only: identify and remove whatever is intercepting pointer events on the anchors inside the mobile drawer (an overlay layer, a stacking-context/z-index gap, or an animation library leaving a transform/pointer-events state applied after the open transition), delete the "Systems" `mobile-menu-link-row` paragraph block, and tighten the padding on the `.ls-simple-submenu-links` / `.is-style-mega-menu-item-service` page-list items inside each accordion, per the theme's existing SCSS-partial and theme.json-first conventions. | ||
|
|
||
| ## Technical Context | ||
|
|
||
| **Language/Version**: PHP 8.x (WordPress block theme), HTML block markup (`parts/*.html`), Sass/SCSS compiled to `assets/css/*.css` | ||
|
|
||
| **Primary Dependencies**: WordPress core Navigation block (`core/navigation`, responsive/overlay container), core `<details>`/`<summary>` accordion block, GSAP (only if the click-blocking root cause turns out to be an animation/transform state — see research.md), no new dependencies planned | ||
|
|
||
| **Storage**: N/A (template part markup + compiled CSS only; no data persistence) | ||
|
|
||
| **Testing**: Manual QA in a browser/device emulator, plus the existing `tests/specs/navigation.spec.ts` Playwright suite (covers mobile menu open/close and accordion toggle at 375px, but not link destinations or 320px — those stay manual); PHP lint / theme.json schema checks per AGENTS.md validation commands; `validate_blocks` tool is banned per project policy — verify block markup via direct JSON/source inspection or the Site Editor instead | ||
|
|
||
| **Target Platform**: WordPress front end, mobile breakpoints (320px, 375px, and general mobile widths below the desktop nav breakpoint) | ||
|
|
||
| **Project Type**: WordPress block theme (single theme codebase, no frontend/backend split) | ||
|
|
||
| **Performance Goals**: No regression to existing mobile menu open/close animation performance; no additional render-blocking assets | ||
|
|
||
| **Constraints**: Theme-first approach — prefer `theme.json` / `styles/**` JSON partials over hand-authored SCSS wherever a JSON-registrable hook exists (per AGENTS.md); SCSS-only for anything JSON cannot express (e.g. core-generated `.wp-block-navigation__responsive-container*` markup, which has no registrable block-style slug); no hardcoded colors — reuse existing semantic tokens; must not regress WCAG 2.2 AA tap-target sizing when reducing padding | ||
|
|
||
| **Scale/Scope**: Single template part (`parts/mobile-menu.html`) plus its supporting SCSS partials (`src/scss/structural/_mobile-menu.scss`, `src/scss/animations/_mobile-menu-motion.scss`, `src/scss/animations/_details-motion.scss`, and possibly `_menu-motion.scss`/`_header-motion.scss` if the click-blocking cause lives in the shared open/close transition); no new template parts or patterns required | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| ## Constitution Check | ||
|
|
||
| *GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.* | ||
|
|
||
| `.specify/memory/constitution.md` is still the unpopulated template (no ratified project-specific principles). This repository's actual governing document is [`AGENTS.md`](../../AGENTS.md), referenced from `CLAUDE.md`. Gates evaluated against it: | ||
|
|
||
| - **Theme-first approach**: PASS (planned) — padding/styling changes will prefer `theme.json`/`styles/**` JSON partials; SCSS is reserved for the core-generated overlay markup that has no JSON-registrable slug, consistent with existing comments in `_mobile-menu-motion.scss`. | ||
| - **SCSS-only, no hand-authored plain CSS**: PASS (planned) — any new/changed rules go into the existing partials under `src/scss/`, not `assets/css/*.css` directly. | ||
| - **No hardcoded colors, semantic token reuse**: PASS (planned) — no new colors are introduced by this fix; existing spacing tokens (`--wp--preset--spacing--*`) will be reused for padding reduction. | ||
| - **PHP Minimalism / no unnecessary abstraction**: PASS (planned) — this is a targeted markup removal + CSS fix, no new PHP logic, patterns, or blocks. | ||
| - **Accessibility (WCAG 2.2 AA)**: GATE — reduced padding must not shrink tap targets below the theme's existing `--wp--custom--spacing--tap-target-min` token or equivalent; verify during implementation. | ||
| - **`validate_blocks` tool banned**: PASS (planned) — verification will use JSON/source inspection and manual Site Editor checks only, per project memory. | ||
|
|
||
| No violations requiring justification. Re-checked after Phase 1 design below — still PASS, no new entities, contracts, or dependencies introduced. | ||
|
|
||
| ## Project Structure | ||
|
|
||
| ### Documentation (this feature) | ||
|
|
||
| ```text | ||
| specs/001-fix-mobile-menu/ | ||
| ├── plan.md # This file (/speckit-plan command output) | ||
| ├── research.md # Phase 0 output (/speckit-plan command) | ||
| ├── data-model.md # Phase 1 output (/speckit-plan command) | ||
| ├── quickstart.md # Phase 1 output (/speckit-plan command) | ||
| ├── contracts/ # Phase 1 output (/speckit-plan command) — not applicable, see note below | ||
| └── tasks.md # Phase 2 output (/speckit-tasks command - NOT created by /speckit-plan) | ||
| ``` | ||
|
|
||
| ### Source Code (repository root) | ||
|
|
||
| This git repository's root **is** the theme root (`parts/`, `src/`, `styles/` sit directly at | ||
| the top level) — there is no nested `wp-content/themes/ls-theme/` path inside the repo itself; | ||
| that only describes where this checkout happens to sit inside a local WordPress install. | ||
|
|
||
| ```text | ||
| parts/ | ||
| └── mobile-menu.html # Template part: accordion labels, page-list links, | ||
| # "Systems" row (removed) | ||
| src/scss/structural/ | ||
| └── _mega-menu.scss # Single-column list layout, mobile-only padding | ||
| # override, tap-target notes (Work/Solutions/Pricing/ | ||
| # Insights/About + Services phase links) | ||
| styles/blocks/ | ||
| ├── details/mobile-menu-accordion.json # Accordion label link styling/focus states | ||
| └── groups/mega-menu-item-service.json # Shared page-list-row style (mobile + desktop) | ||
| assets/css/ | ||
| └── animations.css # Compiled build output — never hand-edited | ||
| ``` | ||
|
|
||
| **Structure Decision**: This is a single WordPress block theme codebase (no frontend/backend split, no | ||
| new project). The fix touches one template part (`parts/mobile-menu.html`), its existing SCSS | ||
| partials, and — where a registrable block-style slug already exists (e.g. `is-style-mobile-menu-accordion`, | ||
| `is-style-mega-menu-item-service`) — the corresponding `styles/**` JSON partial, per the theme's | ||
| theme-first / JSON-first convention. No new files, directories, patterns, or dependencies are | ||
| introduced. A `contracts/` directory is not applicable — this feature exposes no API, CLI, or | ||
| service interface; the "quickstart" instead documents manual browser verification steps. | ||
|
|
||
| ## Complexity Tracking | ||
|
|
||
| *No Constitution Check violations — this section is not applicable.* | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| # Quickstart: Validate the Mobile Menu Fix (LS-3222) | ||
|
|
||
| This is a manual verification guide — this feature has no API/CLI/service contract to test against | ||
| (see [plan.md](./plan.md) Project Structure note), so validation is done in a browser/device emulator | ||
| against the dev environment. | ||
|
|
||
| ## Prerequisites | ||
|
|
||
| - Local or dev WordPress environment running `ls-theme` with the changes from this feature applied. | ||
| - A way to emulate mobile viewports: browser DevTools device toolbar (Chrome/Firefox/Safari) at | ||
| minimum 320px and 375px widths, plus one real mobile device if available. | ||
| - Browser DevTools console open to watch for JS errors. | ||
|
|
||
| ## Setup | ||
|
|
||
| 1. Check out/pull the branch for this feature (`001-fix-mobile-menu` / the Linear branch | ||
| `feature/ls-3222-fix-mobile-menu-restore-links-and-remove-systems`). | ||
| 2. Ensure theme assets are built if `assets/css/*.css` is generated from `src/scss/**` | ||
| (follow the repo's existing build command — do not hand-edit `assets/css/*.css`). | ||
| 3. Load the site's home page (or any page with the global header) in the browser. | ||
|
|
||
| ## Validation Scenarios | ||
|
|
||
| ### 1. Mobile menu links are clickable (FR-001, FR-002, SC-001) | ||
|
|
||
| 1. Resize the viewport to 375px, then 320px. | ||
| 2. Open the mobile menu (hamburger/menu toggle in the header). | ||
| 3. For each top-level accordion (Work, Solutions, Services, Pricing, Insights, About): | ||
| - Tap/click the row *away from the label text* (e.g. the chevron, or the row's empty | ||
| whitespace) to expand it — the label itself is now a link to that section's overview page, | ||
| so tapping the label text will navigate away instead of expanding. | ||
| - **Expected**: the dropdown expands without navigating anywhere. | ||
| - Tap/click the label text itself. | ||
| - **Expected**: navigates directly to that section's overview page (`/work/`, `/solutions/`, | ||
| `/services/`, `/pricing/`, `/blog/`, `/about/`) without needing to open the dropdown first. | ||
| - Re-open the dropdown and tap/click every link inside the page-list. | ||
| - Tap/click the trailing "See all …" link. | ||
| - **Expected**: each tap navigates to the linked page. No dead taps. | ||
| 4. Tap/click the standalone "Contact" link row. | ||
| - **Expected**: navigates to `/contact/`. | ||
| 5. Tap/click both action buttons ("Start a project", "Explore case studies"). | ||
| - **Expected**: each navigates correctly. | ||
|
|
||
| ### 2. "Systems" item is gone (FR-003, FR-007, SC-002) | ||
|
|
||
| 1. With the mobile menu open at 375px and 320px, scan every top-level row and every expanded | ||
| accordion's page list. | ||
| - **Expected**: no "Systems" label or `/systems/` link appears anywhere. | ||
| 2. Confirm there is no empty gap, stray divider, or broken spacing where the row used to sit | ||
| (between "Services" and "Pricing"). | ||
|
|
||
| ### 3. No console errors (FR-005, SC-003) | ||
|
|
||
| 1. With DevTools console open, repeat scenario 1 (open menu, expand each accordion, tap several | ||
| links). | ||
| - **Expected**: zero new errors or warnings logged as a result of these interactions. | ||
|
|
||
| ### 4. Reduced, still-tappable padding (FR-004, SC-004) | ||
|
|
||
| 1. At 320px and 375px, expand any accordion's single-column page-list. | ||
| 2. Visually compare item padding against the pre-fix baseline (or against the values recorded in | ||
| [research.md](./research.md) R3, e.g. `styles/blocks/groups/mega-menu-item-service.json`). | ||
| - **Expected**: visibly tighter padding than before, and adequate vertical spacing between | ||
| consecutive rows in the single-column list. | ||
| 3. Attempt to tap each link, including links immediately above and below one another in the list. | ||
| - **Expected**: no accidental mis-taps on a neighboring link; every tap target still feels | ||
| comfortably sized (informal WCAG 2.2 AA target-size sanity check — 24×24 CSS px minimum). | ||
|
|
||
| ### 5. Breakpoint sanity (FR-006) | ||
|
|
||
| - Repeat scenarios 1–4 at exactly 320px and exactly 375px (not just "mobile-ish" widths), since | ||
| these are the two acceptance-criteria breakpoints called out in the Linear issue. | ||
|
|
||
| ## Sign-off | ||
|
|
||
| All five scenarios passing on dev, across both 320px and 375px, with a clean console, is the | ||
| Definition of Done bar for LS-3222's client-facing acceptance criteria. PR review and merge to | ||
| `develop` follow the repo's normal PR process (see AGENTS.md). |
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.