pal-e-docs

Review: Canvas view with activity-based card sizing

review-1406-2026-06-09 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type -- Feature
  • [x] Lineage -- depends on #36
  • [x] Repo -- ldraney/palinks
  • [x] User Story
  • [x] Context
  • [x] File Targets
  • [x] Feature Flag -- canvas_layout, global, disabled, superadmin only
  • [x] Acceptance Criteria -- 6 items
  • [x] Test Expectations
  • [x] Constraints
  • [x] Checklist
  • [x] Related

Traceability

  • [x] story:canvas-layout label -- present on board item #1406
  • [ ] story note MISSING -- story:canvas-layout is NOT listed in the project-palinks user-stories section. Only containerize-palinks, sortable-links, auth-roles, and custom-domain are listed. [SCOPE] Create user story entry on project-palinks user-stories section for story:canvas-layout.
  • [x] arch:palinks label -- present on board item
  • [ ] arch note MISSING -- no arch-palinks note exists in pal-e-docs. Known board-wide gap acknowledged in #36 review; not blocking individual tickets.
  • [x] Forgejo issue -- ldraney/palinks#40, open

File Targets

  • [x] db/migrate/*_add_canvas_position_to_links.rb -- new file to create, correct (no canvas_x/canvas_y columns exist yet)
  • [x] app/models/link.rb -- verified exists (380 bytes), currently has position/favorite scopes, no click count logic yet. Correct target for canvas scale method.
  • [x] app/views/links/index.html.erb -- verified exists, currently renders div.link-grid with data-controller="sortable". Correct target for canvas container replacement.
  • [x] app/views/links/_link.html.erb -- verified exists (40 lines), renders div.link-card with sortable targets. Correct target for inline position/scale styles.
  • [x] app/javascript/controllers/canvas_controller.js -- new file to create. Controllers directory exists with sortable_controller.js as pattern reference.
  • [x] app/assets/stylesheets/application.css -- verified exists (11KB), has .link-grid and .link-card styles. Correct target for canvas container styles.
  • [x] Do NOT touch: sortable_controller.js -- verified exists (230 lines), legacy grid ordering. Correct exclusion.

Repo Placement

Correct. Issue is filed on ldraney/palinks, all file targets are within the same repo. Single-repo change.

Dependencies

  • #36 (clicks table) -- in todo column on board-palinks (item #1400). This ticket explicitly depends on it for click count data used in scale calculations. The dependency is correctly documented in both Lineage and Constraints sections. #36 must be merged before this ticket can proceed.
  • #34 (feature_flags table and admin UI) -- in backlog column (item #1398). The ticket specifies gating behind a canvas_layout feature flag using feature_enabled? helper, but the feature flag system does not yet exist. Per docs/feature-flags.md: "Feature flag system pending implementation." This is an undocumented dependency.
  • #41 (link groups) -- in todo column (item #1407), shares story:canvas-layout label. Appears to be a follow-on ticket that depends on the canvas view existing first. Not a blocker for this ticket.

Acceptance Criteria

  • [x] "Links render on a canvas with absolute positioning instead of a grid" -- testable, clear
  • [x] "Card size scales proportionally to click count" -- testable, with min/max bounds specified in Constraints (0.7-1.5)
  • [x] "Cards can be dragged to new positions, position persists on reload" -- testable via integration test
  • [x] "Canvas supports pan and zoom" -- testable but broad; could benefit from specifying interaction method (scroll wheel? pinch? buttons?)
  • [x] "Zero-click links render at minimum base size" -- testable, clear edge case
  • [ ] "Feature is gated behind canvas_layout flag, grid remains default" -- NOT testable until #34 (feature flag system) is implemented. The feature_enabled? helper referenced in docs/feature-flags.md does not exist yet.
Missing criteria: No AC for what happens when the feature flag is off (should render original grid). No AC for initial canvas_x/canvas_y placement algorithm for existing links that have no position data.

Blast Radius

  • The sortable_controller.js (230 lines) manages the current grid drag-and-drop. The ticket correctly excludes it, but the canvas_controller.js will need to coexist alongside it since the flag gates which view renders. No conflict as long as the two controllers attach to different DOM containers.
  • No other services or repos affected -- palinks is a standalone Rails app.
  • The .link-grid CSS class is used only in index.html.erb, so replacing it with a canvas container is safe.

Decomposition Assessment

6 file targets in 1 repo, 6 acceptance criteria. This is at the boundary of the 5-minute rule thresholds but all targets are tightly coupled within a single feature. The migration, model method, view changes, Stimulus controller, and CSS form a natural unit. No decomposition needed, provided the feature flag dependency is resolved by either (a) implementing #34 first or (b) using a simpler interim flag mechanism in this ticket's scope.

Recommendations

  • [SCOPE] Add story:canvas-layout entry to project-palinks user-stories section. Suggested text: story:canvas-layout -- Activity-based spatial canvas layout for link cards.
  • [BODY] Add #34 (feature_flags table) as an explicit dependency in the Lineage and Constraints sections, or remove the feature flag AC and simplify to a direct boolean/env-var gate until the flag system exists.
  • [BODY] Add AC for initial placement: "Existing links without canvas positions are placed in a default grid-like arrangement on first canvas render."
  • [BODY] Clarify pan/zoom interaction methods in AC #4 (e.g., "scroll wheel to zoom, click-drag on empty space to pan").