Add custom select component by piotrsac · Pull Request #259 · software-mansion/voyager

piotrsac · 2026-09-21T13:38:20Z

Adds custom select component, styled to be matching to the app UI.
Reviewer is welcome to leave any design comments.

piotrsac · 2026-09-21T13:38:29Z

@claude review

github-actions · 2026-09-21T13:38:46Z

Claude finished @piotrsac's task in 4m 32s —— View job

🦀 Review

Severity: 🔴 blocking · 🟡 should-fix · 🟢 nit

🔴 lib/voyager_web/components/core_components.ex:191 — options are sr-only + tabindex="-1" with no keydown handling, so the new select is pointer-only; every control it replaces (ETS filters, process limit, per-page, refresh interval) was keyboard-operable as a native <select>. Trigger also lacks aria-haspopup/aria-expanded and any name beyond the current value.
🟡 assets/js/hooks/select.js:8 — change is the only inside-panel close trigger, so re-picking the already-selected option fires nothing and leaves the dropdown stuck open. Close on click inside .dropdown-content.
🟢 lib/voyager_web/components/core_components.ex:134 — attr :name, :any where the value is always a string.

<verdict: 1 blocking, 1 should-fix, 1 nit>
· branch piotrsaczawa/voy-273-create-custom-select-component

piotrsac · 2026-09-21T14:08:48Z

@claude review

github-actions · 2026-09-21T14:09:02Z

Claude finished @piotrsac's task in 4m 55s —— View job

🦀 Review

Severity: 🔴 blocking · 🟡 should-fix · 🟢 nit

🟡 lib/voyager_web/components/process_components.ex:83 — the <fieldset disabled={@loading?}> disabled the <select> this replaces; a div role="button" is not a form control, so during a fetch the Limit dropdown still opens while its (disabled) radios drop every pick. Give select/1 a disabled attr like multiselect/1 has.
🟡 lib/voyager_web/components/core_components.ex:168 — @id is on a div, so every <label for=…> aimed at it is inert: ets_table_components.ex:171, process_components.ex:129, data_table_components.ex:284 and the new for={@id} at core_components.ex:251. The three ETS filters announce as buttons named "Any". Add an aria-labelledby/aria-label attr on the trigger.
🟡 assets/js/hooks/select.js:21 — with the radios back in the tab order, change fires on Arrow keys, so ↓ picks the next option, closes the panel and blurs focus to <body>. _onClick already covers pointer picks; drop the change listener and return focus to the trigger on close.
🟢 lib/voyager_web/components/core_components.ex:176 — trigger says aria-haspopup="listbox" but the panel has no role or name; multiselect/1 sets role="group" aria-label.

<verdict: 0 blocking, 3 should-fix, 1 nit>
· branch piotrsaczawa/voy-273-create-custom-select-component

…losing it

piotrsac · 2026-09-21T15:08:39Z

@claude review

github-actions · 2026-09-21T15:08:58Z

Claude finished @piotrsac's task in 4m 10s —— View job

🦀 Review

Severity: 🔴 blocking · 🟡 should-fix · 🟢 nit

🟡 assets/js/hooks/select.js:51 — the click listener is on this.el, and <details open> does not close on an outside click the way :focus-within does for multiselect/1. Click away from an open filter and the panel stays open; open a second select and both overlap. Dismiss from a document listener and remove it in destroyed().
🟡 lib/voyager_web/components/core_components.ex:264 — @id is a <summary>, which is not labelable, so for= is inert: clicking the label no longer opens the control as it did with the <select>. Same at ets_table_components.ex:173, process_components.ex:136, data_table_components.ex:286.
🟢 lib/voyager_web/components/core_components.ex:196 — JS.remove_attribute("open", …) duplicates the hook's _onClick, which already closes on any click inside .dropdown-content.
🟢 lib/voyager_web/components/core_components.ex:175 — aria-haspopup="listbox" on a popup that is role="radiogroup".

<verdict: 0 blocking, 2 should-fix, 2 nits>
· branch piotrsaczawa/voy-273-create-custom-select-component

piotrsac · 2026-09-21T16:05:35Z

@claude review

github-actions · 2026-09-21T16:05:52Z

Claude finished @piotrsac's task in 5m 34s —— View job

🦀 Review

Severity: 🔴 blocking · 🟡 should-fix · 🟢 nit

🟡 assets/js/hooks/select.js:68 — open/aria-expanded are DOM attributes the server never renders, so morphdom clears them on the next LiveView patch and an open dropdown snaps shut on an auto-refresh tick or a validate re-render. Restore them in an updated() hook.
🟢 lib/voyager_web/components/core_components.ex:177 — aria-disabled={@disabled} renders the attribute bare; ARIA reads the empty value as "false". Use @disabled && "true".

<verdict: 0 blocking, 1 should-fix, 1 nit>

piotrsac · 2026-09-21T16:28:19Z

@claude review

github-actions · 2026-09-21T16:28:34Z

Claude finished @piotrsac's task in 6m 46s —— View job

🦀 Review

Severity: 🔴 blocking · 🟡 should-fix · 🟢 nit

🟡 assets/js/hooks/select.js:30 — the _onToggle focus-restore branch is unreachable: toggle fires as a queued task after the radio has already been blurred to <body>, and on the pointer path activeElement is the <summary> the guard excludes. Escape on an open select drops focus to the document. Refocus the trigger in dismiss() before flipping el.open.
🟢 assets/css/styles/select.css:11 — this PR removed the last native <select>, so .select-caret .select:not([multiple]) and the wrapper paragraph above it are dead.

<verdict: 0 blocking, 1 should-fix, 1 nit>
· branch piotrsaczawa/voy-273-create-custom-select-component ·

Closing on the queued toggle event ran after the radio had already blurred, so the next Tab started at the top of the page.

dazakdev · 2026-09-22T14:51:24Z

maybe instead of select.js it would be better to use the LiveView JS commands, as that would make for overall less code for the same features.

hhubert6

I would keep the look similar to default one from daisy ui - keep check mark on the selected option and background color on hover, now it's hardly visible.

…mponent Keep the custom select and point the refresh-interval localStorage hook at its radios.

piotrsac · 2026-09-23T14:30:39Z

I would keep the look similar to default one from daisy ui - keep check mark on the selected option and background color on hover, now it's hardly visible.

@hhubert6 matched the design, slightly widened the interval selects to match the dropdown width

piotrsac added 3 commits September 21, 2026 13:56

add custom select component

75a4ac2

fix display:none override

38698e7

second click closes dropdown menu

9805104

piotrsac changed the title ~~Piotrsaczawa/voy 273 create custom select component~~ Add custom select component Sep 21, 2026