-
Notifications
You must be signed in to change notification settings - Fork 425
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
Add modifier to filter keyboard events. #442
Conversation
This is very cool, and much simpler than I had imagined. Please do proceed. I'd like to see that in Stimulus ❤️ |
@dhh Let me know what you think. |
The modifier was in the wrong position. It should be before the target. * before: evt@global.modifier->... * after: evt.modifier@global->...
before:`${event}@${target}.${filter}->${identifier}#${methodName}` after: `${event}.${filter}@${target}->${identifier}#${methodName}`
Haven't forgotten about this. Just want to get 3.0 out the door first, then revisit this for 3.1. Still thinking and weighing the very nice implementation and syntax of this PR against the limited scope. |
@dhh Don't worry, I'm in no hurry. |
src/core/action.ts
Outdated
@@ -2,6 +2,19 @@ import { ActionDescriptor, parseActionDescriptorString, stringifyEventTarget } f | |||
import { Token } from "../mutation-observers" | |||
import { camelize } from "./string_helpers" | |||
|
|||
const keyMappings: { [key: string]: string } = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this possibility be put on the Stimulus application? Maybe this way the mapping can be modified on a per application basis this way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, the comment was not clear.
The use case would be running Stimulus but being able to pass in a custom (or extended) keyMappings
object when initialising the application (maybe as part of the schema) https://github.com/hotwired/stimulus/blob/main/src/core/schema.ts
// src/application.js
import { Application, defaultSchema } from "@hotwired/stimulus"
const customSchema = {
...defaultSchema,
keyMappings: {...defaultSchema.keyMappings, a: "a", w: "w", s: "s", d: "d" },
}
window.Stimulus = Application.start(document.documentElement, customSchema);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for explaining it all so well! ❤️
I agree with your suggestion.
Give me a minute. :)
…Takuya/stimulus into keyboard-event-modifier
@lb- |
@NakajimaTakuya Do you think it would be a good idea to also make the modifier accept an integer value? Like Edit: Okay I now see that custom mapping can be passed to the |
@radiantshaw I'm assuming that taking an integer value means using KeyboardEvent.keyCode? I think that's already deprecated by the spec. Also, with or without it, I'm not sure I'd be motivated to adopt it too aggressively since it depends on the physical keys on the keyboard. |
@NakajimaTakuya Yeah you're right. Sorry I didn't know about the deprecations. Seems like we're good. But on a separate note, I was thinking if it's possible to also have syntax for |
@radiantshaw |
I found one issue as soon as I started my research. <td role="gridcell"
tabindex="0"
data-action=""
keydown.home->grid#focusFirstCellInRow
keydown.ctrl.home->grid#focusFirstCellInGrid
"
...
> If
Some other libraries seem to default to the former behavior, and then add a .extract modifier to make it behave in the latter way. vue's document: .exact Modifier That seems somewhat complicated to me. I would like to hear what the owners think. |
Reading through this, I think it looks pretty powerful from my perspective and gives some room to add more functionality later. Thanks for taking on some ideas and running with it @NakajimaTakuya |
@lb- Thanks for the review :) |
On second thought, the latter proposal breaks backwards compatibility.
If the existing code had the following implementation, the code that wanted the <td role="gridcell"
tabindex="0"
data-action="
keydown.home->grid#focusFirstCellInGrid
...
"
...
> focusFirstCellInGrid(evt) {
if (!evt.ctrlKey) {
return;
}
....
} The only way out seems to be to support the |
Co-authored-by: LB (Ben Johnston) <mail@lb.ee>
Co-authored-by: LB (Ben Johnston) <mail@lb.ee>
Co-authored-by: LB (Ben Johnston) <mail@lb.ee>
Co-authored-by: LB (Ben Johnston) <mail@lb.ee>
Co-authored-by: LB (Ben Johnston) <mail@lb.ee>
Filter | Key Name | ||
-------- | -------- | ||
enter | Enter | ||
tab | Tab | ||
esc | Escape | ||
space | " " | ||
up | ArrowUp | ||
down | ArrowDown | ||
left | ArrowLeft | ||
right | ArrowRight | ||
home | Home | ||
end | End | ||
[a-z] | [a-z] | ||
[0-9] | [0-9] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A suggestion for refinement - using backticks on the code bits.
| Filter | Key Name |
| ------- | ---------- |
| `enter` | Enter |
| `tab` | Tab |
| `esc` | Escape |
| `space` | " " |
| `up` | ArrowUp |
| `down` | ArrowDown |
| `left` | ArrowLeft |
| `right` | ArrowRight |
| `home` | Home |
| `end` | End |
| [a-z] | [a-z] |
| [0-9] | [0-9] |
OR - if the side borders is not really what we do (by the looks of things).
Filter | Key Name
------- | ----------
`enter` | Enter
`tab` | Tab
`esc` | Escape
`space` | " "
`up` | ArrowUp
`down` | ArrowDown
`left` | ArrowLeft
`right` | ArrowRight
`home` | Home
`end` | End
[a-z] | [a-z
[0-9] | [0-9
<div data-action="keydown.shift+a->listbox#selectAll" role="option" tabindex="0">...</div> | ||
``` | ||
|
||
The list of supported modifier keys is shown below. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry, my bad - I was confused, I will suggest a table approach in a min if that is ok.
Co-authored-by: LB (Ben Johnston) <mail@lb.ee>
Co-authored-by: LB (Ben Johnston) <mail@lb.ee>
Co-authored-by: LB (Ben Johnston) <mail@lb.ee>
Co-authored-by: LB (Ben Johnston) <mail@lb.ee>
Co-authored-by: LB (Ben Johnston) <mail@lb.ee>
I found an issue. This PR break bootstrap events, like class ZIndexController extends Stimulus.Controller {
static values = { step: { type: Number, default: 1 } }
increment(e) {
const by = e.params.by ? Number(e.params.by) : this.stepValue
this.value += by
}
decrement(e) {
const by = e.params.by ? Number(e.params.by) : this.stepValue
this.value -= by
}
set value(val) {
this.element.style.zIndex = Number(val)
}
get value() {
return Number(getComputedStyle(this.element).zIndex)
}
}) <div data-controller="z-index">
<div class="dropdown">
<button type="button" aria-expanded="false" data-bs-toggle="dropdown" data-action="shown.bs.dropdown->z-index#increment hidden.bs-dropdown->z-index#decrement">
<div class="dropdown-menu">
...
</div>
</div>
</div>
|
@NakajimaTakuya may I suggest you raise a standalone issue for this. |
@songjiz One solution I can think of is adding Stimulus-specific escape characters. For example, if you want the |
@songjiz |
@NakajimaTakuya I think |
There are certainly a few widgets that have page up/page down assignments! |
I created PR #677 |
I think it would be interesting to add Current, it would be like this <button data-action="keydown.a->menu#log keydown.b->menu#log keydown.c->menu#log keydown.d->menu#log ..."></button> I wish I had something like this <button data-action="keydown.[a-z]->menu#log"></button> |
@NakajimaTakuya I tried to implement the filter to include all letters from a to z without having to type 26 actions. Unfortunately, I don't see how to do it. Would you have an idea? |
from #440
There are many cases where we want to support keyboard operations as part of accessibility.
In such cases, determining which key is pressed each time and filtering it in the function may promote a divergence between the function name and the behavior when adhering to the teaching that declarative function names should be given.
So, I thought it might be possible to express this filtering on the HTML side to make the function implementation more straightforward.
I know that there is a third party library that does this filter on the HTML side, using a different attribute value than data-action.
However, I'm hoping that this can be integrated into data-action as a Stimulus feature to make this simpler to achieve.
Here is a mapping of the keys supported by the filter modifier.
(I selected the keys that are often used mainly for accessibility support.)