Skip to content

The parameters range slider never reaches its handler #133

Description

@jimsynz

The range slider in the parameters widget cannot work. Found while fixing the
unit-typed parameter crash; it is independent of that bug and predates it, so
it is filed separately rather than folded into that change.

The wiring

render_slider_input/1 puts the hook on the wrapping element:

<div class="bb-slider-input" phx-hook="DebouncedSlider" id={"slider-#{@path_str}"}>
  <input type="range" ... data-path={@path_str} phx-target={@myself} />

assets/js/hooks/debounced_slider.js then reads its own element's dataset:

this.pushEvent("slider_change", {
  name: this.el.dataset.name,
  value: parseFloat(e.target.value),
});

Three things are wrong at once:

  1. this.el is the div, not the input. data-path is on the inner
    range input, so the hook element has no data attributes at all.
  2. data-name is never set anywhere in the repo. this.el.dataset.name is
    undefined, so the payload is {name: undefined, value: 0.5}.
  3. The event never reaches the component. phx-target={@myself} is on the
    inner input; the hook element has no phx-target, so pushEvent from the
    hook goes to the parent DashboardLive. DashboardLive defines no
    handle_event/3 clauses at all, so a slider_change that did fire would
    crash the LiveView rather than be ignored.

Meanwhile handle_event("slider_change", params, socket) on the component
reads params["path"], which the hook never sends.

Consequence

Dragging the range slider does nothing. The number input beside it still works,
because it uses phx-change="set_parameter" with phx-target={@myself} and
does not involve the hook at all — which is presumably why this went unnoticed.

handle_event("slider_change", …) is unreachable dead code as things stand.

Fixing it

Either move the hook onto the range input itself so this.el.dataset.path is
populated, or have the hook read the input it wraps. Either way the payload key
has to become path to match the handler, and the event has to be targeted at
the component — a phx-target on the hook element, or pushEventTo.

Worth deciding at the same time whether slider_change should exist separately
from set_parameter: the two handlers have identical bodies, and the range
input and number input are two views of the same value.

Testing

Nothing currently exercises this. mix test passes with the slider inert,
because no test drives the hook and the component's own tests can call
handle_event/3 directly with whatever params they like. A regression test
needs to assert the params the template actually produces, not the params the
handler happens to want.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions