pal-e-docs

Review: Landing page: Android badge, hamburger nav, web demo pitch

review-1987-2026-08-02b Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type
  • [x] Lineage
  • [x] Repo
  • [x] User Story
  • [x] Context
  • [x] File Targets
  • [x] Feature Flag
  • [x] Acceptance Criteria
  • [x] Test Expectations
  • [x] Constraints
  • [x] Checklist
  • [x] Related

Traceability

  • [x] story:app-store-submission label — "Submit and maintain iOS App Store listing"
  • [x] story note verified — found in project-landscaping-assistant user-stories section
  • [x] arch:rails-frontend label — "Rails Frontend Architecture"
  • [x] arch note verified — arch-rails-frontend note exists in pal-e-docs
  • [x] Forgejo issue — ldraney/landscaping-assistant#355, open

File Targets

  • [x] app/views/landing/index.html.erb — verified: exists (5.2k), hamburger markup present at lines 11-21 as claimed, JS toggle at lines 105-117
  • [x] app/views/landing/_app_store_badge.html.erb — correctly marked as CREATE target; file does not exist yet
  • [x] app/assets/stylesheets/application.css — verified: exists (74k), has .landing-hero/.landing-features/.landing-roles/.landing-footer styles but ZERO styles for .landing-header, .landing-menu-btn, .landing-menu, .landing-logo (confirms ticket claim "zero CSS styles")
  • [ ] app/controllers/landing_controller.rb — ISSUE: not listed in File Targets but likely needs modification. AC2 says hamburger should have "links to Login and Register" — the Keycloak registration URL (@registration_url) is only exposed in SessionsController, not LandingController. If the Register link needs a direct Keycloak URL, the controller must be modified.

Repo Placement

OK — issue filed on ldraney/landscaping-assistant, all file targets are in the same repo. Single-repo change.

Dependencies

  • #1988 "Mobile app: skip landing page, go straight to login/register" (backlog, same story:app-store-submission) — related but not blocking. The landing page is web-only; Turbo Native users bypass it per #1988's scope. No dependency conflict.
  • #1983 and #1984 (QA, story:app-store-submission) — no direct dependency on this work.
  • No items in in_progress block this ticket.

Acceptance Criteria

  • 6 ACs — all are verifiable via visual inspection
  • AC2 is ambiguous: "links to Login and Register" — does Register mean a direct Keycloak registration URL (requires controller change not in scope) or just /login (where "Create Account" already exists, making a separate link redundant)?
  • AC4 "Mobile downloads remain primary CTA" — subjective but can be verified via visual hierarchy (size, position, color treatment)
  • AC6 "Layout is responsive" — testable at standard breakpoints but no specific breakpoints listed
  • Test Expectations are reasonable and verifiable

Blast Radius

Low. The landing page is a standalone public-facing page routed as the app root (landing#index). It does not use the application layout. No other apps or views share this template. CSS changes use the scoped .landing-* class naming convention, avoiding collisions with authenticated UI styles.

Decomposition Assessment

  • 3 file targets, 1 repo — within threshold
  • 6 acceptance criteria — exceeds the >5 threshold, but all criteria are tightly coupled (same page, same template). Per user preference, keeping unified is appropriate.
  • Estimated agent work: ~5 minutes (inline SVG badges, CSS for hamburger, copy rewrite). Borderline but manageable in one pass.
  • No decomposition needed.

Recommendations

  • [BODY] Fix Context section: it claims "It currently has: 1. An App Store badge linking to the iOS listing" — this is incorrect. The current index.html.erb has NO badge markup whatsoever. The agent must CREATE badges from scratch, not modify existing ones. Also, the statement "The landing page was rebuilt in #349" is misleading — #349 is a closed planning issue with no associated merged PR; the landing page was built by PR #324/#329.
  • [BODY] Add app/controllers/landing_controller.rb to File Targets if Register link should go directly to Keycloak registration URL (currently only available in SessionsController#new via keycloak_registration_url). Alternatively, clarify in AC2 that Register links to /login (which already has a "Create Account" link).