Skip to content

Optional messages for advanced events#366

Closed
Norlock wants to merge 1 commit intolustre-labs:mainfrom
Norlock:optional-events
Closed

Optional messages for advanced events#366
Norlock wants to merge 1 commit intolustre-labs:mainfrom
Norlock:optional-events

Conversation

@Norlock
Copy link

@Norlock Norlock commented Aug 8, 2025

this comes in handy when you need to stop a lot of events from propagating. Also improved the advanced API with evend.send and event.retain which helps reduce boilerplate. Think about a Dialogs component which needs to stop a lot of events from propagating. This also triggers a lot of necessary lustre.view calls.

Example:

let on_keydown = {
    use key <- decode.field("key", decode.string)

    case key {
      "a" ->
        event.send(
          dispatch: OnKeyDown("a"),
          prevent_default: False,
          stop_propagation: False,
        )
      _ -> event.retain(prevent_default: False, stop_propagation: True)
    }
  }

this comes in handy when you need to stop a lot of events from
propagating. Also improved the advanced API with `evend.send` and
`event.retain` which helps reduce boilerplate.
@hayleigh-dot-dev
Copy link
Collaborator

Hey there! I appreciate it takes a little bit of time to put PRs like this together so first of all thank you for opening this. For now I will have to reject this PR however, for a couple of reasons:

  • This would introduce a breaking change. Breaking changes aren't inherently bad, but for projects like Lustre getting the community to move with major version bumps can be slow and it's important we don't unecessarily fragment the community.

    We could work around this by deprecating handler but there are still other issues.

  • Your changes have broken Lustre by removing event mapping. Lustre's internals are written in a way most Gleam code wouldn't be, and we do some things in the name of performance that are unsafe and require careful consideration before modifying.

  • I'm not yet sure that this addition would be a net positive for Lustre applications. In general, side effects in Lustre applications are performed in a controlled manner. We don't have imperative event handlers we have declarative event decoders that are used to communicate with our applications.

    In this case, we are telling Lustre that our application does not wish to receive a message for this event but would still like the runtime to affect the world in some way. This is counter to Lustre's mental model and I would like to see a strong case for its inclusion before I would consider it.


This PR wasn't opened in a vacuum so let me also make some general observations around the thread on discord. You discuss use of native HTML dialog elements and needing to stop propagation of key events when the dialog is open. The behaviour you want already exists in the platform by way of modal dialogs:

The HTML element is used to create both modal and non-modal dialog boxes. Modal dialog boxes interrupt interaction with the rest of the page being inert, while non-modal dialog boxes allow interaction with the rest of the page.

I suspect you are currently toggling the dialog's visibility using the open attribute, but as MDN describes you should use the .showModal() method to open a true modal dialog. This would get you the behaviour you're looking for without requiring changes to Lustre or indeed handling keyboard events you don't care about.

To do this in Lustre I would write a small effect that queries the DOM for the dialog you want to open and then invokes this method, perhaps:

fn open_modal(selector: String) -> Effect(msg) {
  use _, _ <- effect.before_paint
  do_open_modal(selector)
}

@external(javascript, "./ffi.mjs", "open_modal")
fn do_open_modal(selector: String) -> Nil
export const open_modal = selector => {
  const element = document.querySelector(element)

  if (element instanceof HTMLDialogElement) {
    element.showModal()
  }
}

I hope this gets you on the right path, it sounds like exactly what you need! Thank you again for taking the time to open a PR; interest in improving the project is always exciting 💕

Sign up for free to 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