Skip to content

Adds two opt-in stateless helpers to PhoenixKitWeb.Components.Core.TableDefault and migrates two admin pages, fixing a search debounce UX bug along the way. - #528

Merged
ddon merged 7 commits into
BeamLabEU:devfrom
timujinne:dev
May 10, 2026

Conversation

@timujinne

Copy link
Copy Markdown
Contributor

Changes

New components (lib/phoenix_kit_web/components/core/table_default.ex)

  • sort_header_cell/1 — clickable <th> with chevron-up-mini/chevron-down-mini icon when active. Inert label-only <th> when sort attr is nil. Configurable event, target, align (:left/:right/:center).
  • search_toolbar/1 — daisyUI input-sm with hero-magnifying-glass icon and phx-debounce (default 300ms). Optional <form> wrap when on_submit is set. Placeholder defaults to dgettext("default", "Search...").

Migrations

  • live/users/live_sessions.{ex,html.heex} — collapsed :sort_by + :sort_order assigns into a single :sort = %{by, dir} map; renamed event "sort_by""toggle_sort" with "by" param. Replaced inline header-button + chevron blocks and search form with the new
    components.
  • live/users/users.html.heex — replaced bare search form (every keystroke hit the server) with <.search_toolbar>. Bug fix.

Tests

  • New test/phoenix_kit_web/components/core/ directory + table_default_test.exs with 13 tests covering both new components.

i18n

  • Catalog resync via mix gettext.extract --merge (separate commit). Adds "Search..." msgid + et / ru translations; surfaces accumulated drift from prior commits where extract wasn't run.

Test Plan

  • mix precommit green (compile + docs + credo --strict + dialyzer)
  • 13 component tests passing
  • Verified live in Decor 3D Print parent app via Tidewave MCP — both new functions exported and DB schema at v111 (no pending migrations)
  • /admin/users/live-sessions — sort headers + debounced search work in browser
  • /admin/users — debounced search works in browser

Brings the admin permissions fix for external plugin LiveViews
(PR target: BeamLabEU/phoenix_kit:dev) into local dev so future
work on dev is based on the corrected resolver.
# Conflicts:
#	lib/phoenix_kit/module_registry.ex
#	test/phoenix_kit/module_registry_test.exs
- New stateless component sort_header_cell/1: renders a sortable
th-button with chevron-up-mini/chevron-down-mini icon when active.
Inert (label-only th) when sort attr is nil. Configurable event,
target, and align (:left/:right/:center). Active column shows
chevron only — no color or weight change.
- New stateless component search_toolbar/1: daisyUI input-sm with
hero-magnifying-glass icon, phx-debounce default 300ms, optional
form wrapper when on_submit is set. Placeholder defaults to
dgettext("default", "Search...").
- Migrate live_sessions admin page: collapsed :sort_by + :sort_order
assigns into a single :sort map (%{by, dir}), renamed event
"sort_by" -> "toggle_sort" with "by" param, replaced inline header
buttons + chevron blocks with <.sort_header_cell>, replaced search
form with <.search_toolbar>.
- Fix users admin page search debounce: replaced bare form/input
(every keystroke hit the server) with <.search_toolbar>, which
applies phx-debounce=300 by default.
- Add 13 component tests in new test/phoenix_kit_web/components/core/
directory covering both new components.
Re-runs mix gettext.extract --merge to bring priv/gettext in sync
with the current source tree. The change surfaces accumulated drift
from prior commits where extract was not run:
- 75 new msgids picked up (DeepSeek/Mistral providers, Azure OAuth
flow, integration error strings, time-ago helpers, plus our new
Search... msgid for the search_toolbar component).
- 76 obsolete msgids dropped (orphaned customer_service strings
from the extracted module).
- ~530 line-number reference shifts on existing msgids.
Translations added for the new Search... msgid:
- en: "" (falls back to msgid)
- et: "Otsi..."
- ru: "Поиск..."
de/es/fr/it/pl: only the line-number shifts and orphan removals
(no new translations in those locales — left empty for translators).
The mix phoenix_kit.install task copies phoenix_kit.js to a parent
app's priv/static/assets/vendor/ directory. When the task is run
against PhoenixKit itself (e.g. for testing), it creates that path
inside /app, leaving an outdated copy of the source JS in tree.
Add the directory to .gitignore so the stray copy never sneaks into
a commit.
- Drop phx-change from <form> — was duplicating the binding on the
inner <input>, causing the same "search" event to fire twice in
quick succession on each keystroke (input first, then form on the
same change). Server did double work; payloads matched so no
incorrect state, but redundant.
- Add phx-target to <form> so phx-submit retargets correctly when
the toolbar is rendered inside a LiveComponent. Latent bug — no
in-tree consumer used target with on_submit, but the contract was
broken for future ones.
- Add 2 regression tests asserting phx-change appears exactly once
and phx-target appears on both <form> and <input> in the form
variant. Test count now 15 (was 13).
Both issues surfaced in the self-review for PR BeamLabEU#529.
Self-review of PR BeamLabEU#529 archived to dev_docs/pull_requests per
project convention. Verdict: APPROVE with NITPICKs. Two
IMPROVEMENT-MEDIUM items (search_toolbar form variant double-binds
phx-change; phx-target not propagated to <form>) were fixed in
preceding commit dfc9123.

@timujinnetimujinne left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-review (Comment)

Verdict: APPROVE with NITPICKs. Three commits: clean .gitignore + gettext
resync + the feature commit. Feature commit was already through internal
two-stage review that caught and fixed two bugs (dead assign_new placeholder
default, missing mix gettext.extract --merge). Both correctly addressed in
final state.

IMPROVEMENT - MEDIUM — search_toolbar/1 form variant double-binds

phx-change

lib/phoenix_kit_web/components/core/table_default.ex:491,501 — the form variant
places phx-change={@on_change} on both the <form> and the inner
<input>. Phoenix LiveView fires phx-change once per element binding; with
identical events on both you can end up with the same "search" event firing
twice in quick succession (input on keystroke, then form on the same change). The
handler matches both with the same payload — no incorrect state — but it doubles
server work and is conspicuously redundant.

Fix: drop phx-change from <form>, keep only on <input>. The form is
there for phx-submit on Enter, nothing more.

IMPROVEMENT - MEDIUM — phx-target not propagated to <form>

lib/phoenix_kit_web/components/core/table_default.ex:489-506 — when caller
passes <.search_toolbar target="#my-component">, phx-target lands on the
<input> (so phx-change retargets correctly) but not on the <form> (so
phx-submit does not). When the toolbar is rendered inside a LiveComponent,

NITPICK — Behaviour change on first click of a new column

Old live_sessions.ex set new-column click to :desc. New code sets it to
:asc (spec-correct, matches the chevron-up-first convention of the new
component). Two columns, low-traffic admin page — fine; mention in CHANGELOG when
the maintainer writes one.

NITPICK — Chevron icon size/style change

Old: hero-chevron-up/down (24px outline). New: hero-chevron-up-mini/down-mini
(20px solid). Correct call for a header indicator (more compact), but the diff
is silent about it. Future migrations of other sortable headers should follow the
same convention.

NITPICK — Test coverage gaps

Declared but not asserted: target on search_toolbar/1 (would surface the
second MEDIUM above), name custom value, custom debounce, event default on
sort_header_cell/1. Fold into the standing core/ test-coverage TODO from
CLAUDE.md.

NITPICK — HEEX duplication between form/div branches

table_default.ex:489-521<input> markup (~12 lines) is copy-pasted into
both if @on_submit branches. EEx makes wrapper-only conditionals awkward, so
the current shape is readable. Could be tightened later by extracting a private
render_search_input/1 partial; not worth it in this PR.

Things deliberately not flagged

  • Gettext catalog drift (~10000 lines in commit a7c1d35b): explicitly accepted
    as a separate commit doing long-overdue resync.
  • mix.exs @version and CHANGELOG.md untouched per Version + CHANGELOG ownership rule.
  • <th> semantics, active-column styling (chevron only), phx-target={nil}
    auto-omit, CSRF/XSS, .gitignore scope — all correct.

Full archived review with rationale: dev_docs/pull_requests/2026/529-table-defau lt-sort-search-helpers/CLAUDE_REVIEW.md (will be in the PR after the fix
commit).

@ddon
ddon merged commit d0cc931 into BeamLabEU:devMay 10, 2026
ddon pushed a commit that referenced this pull request May 10, 2026
Resolves the items flagged in the PR #528 follow-up review:
- live_sessions pagination crashed on click (phx-click="goto_page"
with no matching handler). Renamed to "change_page" and added the
handler mirroring users.ex.
- sort_header_cell now sets aria-sort on the <th> and lifts the
align attr to the <th class> so non-sortable columns honour it.
- Dropped redundant on_submit="search" from both call sites; the
input's debounced phx-change already covers the same event.
- Tightened flip_dir/1 to explicit :desc clause.
- 5 new tests: aria-sort (ascending/descending/none/omitted) and
align on inert <th>.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ddon pushed a commit that referenced this pull request May 10, 2026
PR #528 (sort+search helpers in TableDefault) and its follow-up:
pagination fix on live_sessions, aria-sort, redundant on_submit
removal, flip_dir tightening, mix.lock dep bumps.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ddon pushed a commit that referenced this pull request May 10, 2026
PR #528 (sort+search helpers in TableDefault) + follow-up fixes
(pagination, aria-sort, on_submit redundancy) + mix.lock dep bumps.
timujinne pushed a commit to timujinne/phoenix_kit that referenced this pull request May 13, 2026
PR BeamLabEU#528 (sort+search helpers in TableDefault) + follow-up fixes
(pagination, aria-sort, on_submit redundancy) + mix.lock dep bumps.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@timujinne@ddon