Fix: autosubmit no longer ignores the space key and some punctuation - #1303
Conversation
isInKeyRange(e, 9, 40) tests e.key.charCodeAt(0), a leftover from the era of e.keyCode, where 9-40 stood for Tab, Enter, Shift, Esc, space and the arrows. Over e.key it is a different quantity, and the named keys it was meant to catch are already handled by the length check above it. All the range still does is keep the space key and !"#$%&'( from triggering the filter, so typing "vitamin " leaves the grid unfiltered until the next character arrives. shouldIgnoreKey() states the rule directly: a printable character always changes the value, a named key never does unless it is Backspace or Delete. isInKeyRange() and isFunctionKey() stay exported, the plugin just no longer uses them.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1303 +/- ##
=======================================
Coverage 49.12% 49.12%
=======================================
Files 63 63
Lines 2974 2974
=======================================
Hits 1461 1461
Misses 1513 1513 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR fixes autosubmit’s key-filtering logic so that typing space and certain punctuation correctly triggers autosubmit, by replacing the legacy “key range” heuristic with an explicit “ignore named keys except editing keys” rule.
Changes:
- Add
shouldIgnoreKey()utility to classify keys to ignore for autosubmit. - Update Autosubmit plugin keyup handling to use
shouldIgnoreKey()instead ofisInKeyRange()/isFunctionKey().
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| assets/utils.ts | Adds shouldIgnoreKey() helper used to decide which keyups should be ignored. |
| assets/plugins/features/autosubmit.ts | Switches autosubmit key filtering to shouldIgnoreKey() to stop ignoring space and punctuation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| export function shouldIgnoreKey(e: KeyboardEvent): boolean { | ||
| if (e.key.length === 1) { // Printable characters always change the input value | ||
| return false; | ||
| } | ||
| return e.key !== "Backspace" && e.key !== "Delete"; | ||
| } |
|
Díky, mergnuto. 🙏 Trefná diagnóza — `isInKeyRange(e, 9, 40)` byl skutečně pozůstatek po `e.keyCode` a moje dvě předchozí záplaty (ae9e9be, c9835f1) ten rozsah jen obcházely, místo aby ho zrušily. `shouldIgnoreKey()` říká pravidlo přímo a je o jednu vrstvu výš, kam patří. Připomínku Copilota k surrogate pairs beru jako teoretickou — emoji picker vkládá text přes `input`/composition, ne přes `keyup`. Samostatně ještě dořeším, že celý přístup přes `keyup` nechytá vložení myší, autofill, IME a na Androidu často přijde `e.key === "Unidentified"`. Správně to patří na `input` event, ale to je na jiný PR. |
isInKeyRange(e, 9, 40) tests e.key.charCodeAt(0), a leftover from the era of e.keyCode, where 9-40 stood for Tab, Enter, Shift, Esc, space and the arrows. Over e.key it is a different quantity, and the named keys it was meant to catch are already handled by the length check above it. All the range still does is keep the space key and !"#$%&'( from triggering the filter, so typing "vitamin " leaves the grid unfiltered until the next character arrives.
shouldIgnoreKey() states the rule directly: a printable character always changes the value, a named key never does unless it is Backspace or Delete. isInKeyRange() and isFunctionKey() stay exported, the plugin just no longer uses them.