Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support keyboard modifier for click events #665

Merged
merged 1 commit into from
May 29, 2023

Conversation

shouichi
Copy link
Contributor

@shouichi shouichi commented Mar 7, 2023

This commit adds functionality to accept modifiers for click events, e.g., ctrl+click->link#open.

This commit adds functionality to accept modifiers for click events,
e.g., `ctrl+click->link#open`.

Co-authored-by: oljfte <oljfte@gmail.com>
@lb-
Copy link
Contributor

lb- commented Mar 7, 2023

Just a thought, this can be done I think pretty easily with custom action options out of the box today. I am wondering of this would be sufficient for your requirements.

I have validated and the below works really well, it even supports the click shorthand data-action notation nicely.

import { Application } from "@hotwired/stimulus"

const application = Application.start();

application.registerActionOption("alt", ({ event, value }) => value ? event.altKey : !event.altKey);
application.registerActionOption("ctrl", ({ event, value }) => value ? event.ctrlKey : !event.ctrlKey);
application.registerActionOption("meta", ({ event, value }) => value ? event.metaKey : !event.metaKey);
<button type="button" data-action="gallery#view!alt gallery#expand:alt">View (using shorthand click action)</button>
<button type="button" data-action="click->gallery#expand:ctrl">View (only activate if ctrl is clicked)</button>
<button type="button" data-action="click->gallery#expand:alt:meta">View (only activate if both alt & meta are pressed)</button>

Related links:

An even shorter version of the action options...

    ['alt', 'ctrl', 'meta'].forEach((key) => {
      application.registerActionOption(
        key,
        ({ event, value, keyPressed = event[`${key}Key`] }) =>
          value ? keyPressed : !keyPressed
      );
    });

@shouichi
Copy link
Contributor Author

shouichi commented Mar 8, 2023

Cool! Do you think adding those custom action options to Stimulus itself is sensible? They seem common enough.

@lb-
Copy link
Contributor

lb- commented Mar 8, 2023

@shouichi I'm not a core maintainer so not sure.

I'd probably recommend raising an issue that explains the reasoning/use cases.

Then raise a new PR with bringing it into core against that issue.

Worst case scenario if ends up being a good reference for people googling the use case.

My personal opinion - it's probably good to have this in core but risks the action options getting a bit too long, plus documentation/core implementation would need to consider the nuance of this not being suitable for some kinds of events (non pointer/ mouse events).

@shouichi
Copy link
Contributor Author

shouichi commented Mar 8, 2023

Thank you for the tip!

@lb-
Copy link
Contributor

lb- commented Mar 8, 2023

Also. Should probably cover shiftKey.

@dhh dhh merged commit 7593959 into hotwired:main May 29, 2023
@dhh
Copy link
Member

dhh commented May 29, 2023

Even if this is possible already by custom action options, I think it's considerably nicer to have part of the core package 👍

@shouichi shouichi deleted the click-modifier branch May 30, 2023 06:09
AndersGM pushed a commit to AndersGM/stimulus that referenced this pull request Jun 16, 2023
This commit adds functionality to accept modifiers for click events,
e.g., `ctrl+click->link#open`.

Co-authored-by: oljfte <oljfte@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants