MediaGallery: add max_count attr + disable Add button at limit

[timujinne]

Summary

PhoenixKitWeb.Components.MediaGallery LiveComponent now accepts an optional :max_count integer attribute. When set, the gallery caps the selection and disables the Add (open picker) button once the limit is reached.

• :single mode implicitly enforces max_count = 1 regardless of the attribute, matching existing behaviour.
• :multiple mode with a positive max_count clamps any selection via apply_selection/4 (defence-in-depth — the disabled button is the primary UI guard) and disables the Add trigger when length(selected) >= max_count.
• nil / unset = unlimited (no behavioural change for existing callers).
• selection_at_limit?/3 mirrors apply_selection so the button state stays in sync with the selection rule.

Used by consumers (e.g. Andi's document picker) that need to honour a template-author-defined cap on the number of images per slot.

Tests

7 focused cases added covering below-limit, at-limit, unlimited, single-mode, and apply_selection clamping. Full suite green locally.

Test plan

• CI green
• Sanity-check a consumer page with max_count={3}: the Add button disables at the third selection and re-enables after a removal
• Confirm :single callers (no max_count set) behave unchanged

[timujinne]

PR #556 Review — table_default: card_media slot, controlled view_mode, header class override

Scope: Only commit 15ddac1e (the table_default changes). Commit 0805bb59 (MediaGallery max_count) is NOT reviewed here.
File:lib/phoenix_kit_web/components/core/table_default.ex
Reviewer: Claude Opus 4.6

---

Backwards Compatibility

Verified all 15 existing usage sites of <.table_default> and <.table_default_header> across lib/. None pass view_mode, view_event, card_media, or class to table_default_header. All defaults preserve existing behavior:

• view_mode=nil → JS hook + localStorage (unchanged)
• table_default_header class defaults to "bg-primary text-primary-content" (unchanged)
• card_media slot empty → no wrapper div rendered (:if guard)
• Toggle buttons: data-view-action still emitted in JS mode, phx-click is false (suppressed by Phoenix)

Verdict: No regressions for existing consumers.

---

Findings

NITPICK: above_cards slot doc mentions only JS hook hiding

File:table_default.ex:171

The above_cards doc says "Hidden automatically when the JS hook switches to table mode". In controlled mode (view_mode="table"), hiding is driven by the CSS class hidden on the parent data-card-view div, not by the JS hook. The behavior is correct either way (slot is hidden in table mode), but the doc is JS-mode-specific. Consider adding a note: "In controlled mode, hidden when view_mode="table" via the parent container."

NITPICK: card_media wrapper div has no class — intentional but worth a doc note

File:table_default.ex:354

The <div :if={@card_media != []}> wrapper is classless. The moduledoc says "slot owns its own padding/background", which is correct — the consumer controls styling. However, since this div sits outside card-body (above it), consumers using daisyUI's figure pattern might expect an auto-applied <figure> tag. Current approach is fine — a plain <div> is more flexible — but confirming this is a deliberate design choice, not an oversight.

NITPICK: Duplicate "grid" class in card-view controlled mode

File:table_default.ex:323-327

class={["grid gap-4 md:grid-cols-2 lg:grid-cols-3 2xl:grid-cols-4",# always has "grid"is_nil(@view_mode)&&"md:hidden",@view_mode=="card"&&"grid",# ← redundant "grid"@view_mode=="table"&&"hidden"]}

The base class list already includes "grid". The @view_mode == "card" && "grid" line is redundant — it adds "grid" again. Harmless (CSS deduplicates), but noisy. Could be removed. Similarly, "block" in the table-view controlled branch (line 300) is redundant since <div> is block by default. These are symmetry/readability choices more than bugs.

---

Technical Verification

• phx-click={@view_mode && @view_event} when view_mode=nil:nil && "switch_view" → nil. Phoenix suppresses nil-valued attributes. No phx-click in DOM. Correct.
• data-view-action={if is_nil(@view_mode), do: "card"} when view_mode is set: returns nil, attribute suppressed. JS hook also suppressed. Consistent.
• values: [nil, "card", "table"]: Phoenix.Component accepts nil in values list — documented pattern for optional enum attrs. Correct.
• No class conflicts in controlled mode: JS hook not attached → no JS toggling md:hidden/md:block. Each div gets exactly one visibility class. No md:-prefixed classes leak into controlled mode. Verified for all three states (nil, "card", "table").
• Compilation: Clean compile, no warnings.

---

Verdict: Approve

Clean, well-scoped extension. All three features (card_media, controlled view_mode, header class override) are backwards-compatible and correctly implemented. No bugs found. Three nitpicks, all optional.

[timujinne]

Review 2: table_default_header default class change (20132de6)

Verdict: Approve — no issues found.

---

Call-site audit

14 call sites use <.table_default_header> without an explicit class= — all will silently adopt bg-base-300. Checked every one:

• activity/index.html.heex:131, users.html.heex:179, sessions.html.heex:215, live_sessions.html.heex:196, roles.html.heex:161, settings/users.html.heex:414, integrations.html.heex:77, jobs/index.html.heex:188,292 (PhoenixKit core)
• referrals/list.html.heex:126, storage/dimensions.html.heex:143,339, storage/settings.html.heex:184, storage/health.html.heex:217 (modules)

None rely on the primary band for intentional UX emphasis — all are standard admin data tables where a calmer header is appropriate.

Contrast

bg-base-300 + inherited base-content is the idiomatic daisyUI pairing and is contrast-safe across all themes (including dark). The <th> cells carry no explicit text color, so they correctly inherit from the page. Actually more robust than the old bg-primary text-primary-content since it stays in the neutral scale.

Documentation

@doc at table_default.ex:440-449 is correctly updated: documents the new default, explains how to restore the legacy look, no stale references to bg-primary as default remain.

Tests

No tests assert on header CSS classes — zero breakage risk.

Clean change, well-documented escape hatch for anyone who wants the old look back.