Uh oh!
There was an error while loading. Please reload this page.
feat(keyboard): add array overload to pressSequentially - #40748
feat(keyboard): add array overload to pressSequentially#40748Dmitry Gozman (dgozman) merged 5 commits into
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| ### option: Frame.press.timeout = %%-input-timeout-js-%% | ||
| * since: v1.8 | ||
| ## async method: Frame.pressSequentially |
There was a problem hiding this comment.
Let's not introduce a discouraged method 😄
There was a problem hiding this comment.
Agreed, removed.
| - `text` <[string]|[Array]<[string]>> | ||
| String of characters to sequentially press into a focused element. | ||
| String of characters to sequentially press into a focused element, or an array of key names to press sequentially. Key names follow the same format as [`method: Locator.press`]. |
There was a problem hiding this comment.
How about we support the following notation: Hello{ArrowRight}{Ctrl+A}world!{Backspace}? Basically, treating anything inside {} as a key from the press method, when passed a namedKeys: true option.
There was a problem hiding this comment.
Reworked. Added namedKeys: true option to Locator.pressSequentially and Keyboard.type. {KeyName} treats brace content as a key name for press(), with {{/}} for literal braces.
awaitlocator.pressSequentially('Hello{Enter}World',{namedKeys: true});awaitlocator.pressSequentially('{Control+A}{Delete}Hello',{namedKeys: true});awaitlocator.pressSequentially('type {{braces}}',{namedKeys: true});// types: type {braces}Add string[] overload to Locator.pressSequentially and new Keyboard.pressSequentially method, as requested in microsoft#40740. When given an array, each element is treated as a key name (same format as keyboard.press()) and pressed sequentially with optional delay between presses. Fixesmicrosoft#40740
Add missing documentation for Frame.pressSequentially selector-based method to fix doclint CI failure. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
Rework pressSequentially based on review feedback:
- Drop Frame.pressSequentially (no new discouraged methods)
- Drop Keyboard.pressSequentially and keyboardPressSequentially protocol
- Add namedKeys option to Locator.pressSequentially and Keyboard.type
- {KeyName} syntax treats braces content as key names for press()
- {{ and }} escape to literal brace characters
Fixes: microsoft#40740aec15c9 to
4375b42Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| - `namedKeys` <[boolean]> | ||
| When `true`, anything inside `{}` in the text is treated as a key name (same format as [`method: Keyboard.press`]), | ||
| allowing you to mix regular characters with special keys like `{Enter}`, `{ArrowDown}`, or modifier combos like `{Control+A}`. |
There was a problem hiding this comment.
Having an example that contains both text and special keys would be nice to explain the feature.
There was a problem hiding this comment.
Done.
| - `text` <[string]> | ||
| String of characters to sequentially press into a focused element. | ||
| String of characters to sequentially press into a focused element. When [`option: namedKeys`] is `true`, anything inside `{}` is treated as a key name (same format as [`method: Locator.press`]). Use `{{` and `}}` to type literal brace characters. |
There was a problem hiding this comment.
| String of characters to sequentially press into a focused element. When [`option: namedKeys`] is `true`, anything inside `{}` is treated as a key name (same format as [`method: Locator.press`]). Use `{{` and `}}` to type literal brace characters. | |
| String of characters to sequentially press into a focused element. When [`option: namedKeys`] is `true`, anything inside `{}` is treated as a key name (same format as [`method: Locator.press`]). |
There was a problem hiding this comment.
Done.
| Focuses the element, and then sends a `keydown`, `keypress`/`input`, and `keyup` event for each character in the text. | ||
| When [`option: namedKeys`] is `true`, anything inside `{}` is treated as a key name (same format as [`method: Locator.press`]). Use `{{` and `}}` to type literal brace characters. |
There was a problem hiding this comment.
| When [`option: namedKeys`] is `true`, anything inside `{}` is treated as a key name (same format as [`method: Locator.press`]). Use `{{` and `}}` to type literal brace characters. | |
| When [`option: namedKeys`] is `true`, anything inside `{}` is treated as a key name (same format as [`method: Locator.press`]). |
There was a problem hiding this comment.
Done.
| Sends a `keydown`, `keypress`/`input`, and `keyup` event for each character in the text. | ||
| When [`option: namedKeys`] is `true`, anything inside `{}` is treated as a key name (same format as [`method: Keyboard.press`]). Use `{{` and `}}` to type literal brace characters. |
There was a problem hiding this comment.
| When [`option: namedKeys`] is `true`, anything inside `{}` is treated as a key name (same format as [`method: Keyboard.press`]). Use `{{` and `}}` to type literal brace characters. | |
| When [`option: namedKeys`] is `true`, anything inside `{}` is treated as a key name (same format as [`method: Keyboard.press`]). |
There was a problem hiding this comment.
Done.
| expect(await page.evaluate(() => document.querySelector('textarea').value)).toBe('Hello {World}'); | ||
| }); | ||
| it('should type with namedKeys and delay', async ({ page }) => { |
There was a problem hiding this comment.
Let's remove this test.
There was a problem hiding this comment.
Done.
| expect(await page.evaluate(() => document.querySelector('textarea').value)).toBe('Hello\nWorld'); | ||
| }); | ||
| it('should type with namedKeys and modifier combos', async ({ page }) => { |
There was a problem hiding this comment.
Let's remove this test.
There was a problem hiding this comment.
Done.
| return result; | ||
| } | ||
| function* parseNamedKeys(text: string): Generator<{ type: 'key' | 'char', value: string }> { |
There was a problem hiding this comment.
Can we avoid a generator here? It's not like it helps with performance or something.
There was a problem hiding this comment.
Done.
| } | ||
| } | ||
| } else if (text[i] === '}') { | ||
| if (i + 1 < text.length && text[i + 1] === '}') { |
There was a problem hiding this comment.
It seems like there is no point to escape } as }} - these both yield the same result. Let's drop that and update docs to only mention {{ for escaping {.
There was a problem hiding this comment.
Done.
| if (delay) | ||
| await progress.wait(delay); | ||
| await this.insertText(progress, char); | ||
| if (options?.namedKeys) { |
There was a problem hiding this comment.
We can combine both branches by making parseNamedKeys return all "chars" when namedKeys option is not passed. This should simplify the code quite a bit.
There was a problem hiding this comment.
Done.
| for (const token of parseNamedKeys(text)) { | ||
| if (token.type === 'key') { | ||
| if (delay) | ||
| await progress.wait(delay); |
There was a problem hiding this comment.
Perhaps just pass delay to this.press() on the next line?
There was a problem hiding this comment.
Done.
- Combine namedKeys/non-namedKeys branches into single code path
- Pass delay to press() for named keys
- Replace generator with regular function returning array
- Drop redundant }} escaping, keep only {{ for literal brace
- Simplify docs wording per reviewer suggestions
- Add mixed text+key example to Keyboard.type docs
- Trim tests to two meaningful cases using textarea
Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Dmitry Gozman (dgozman)
left a comment
There was a problem hiding this comment.
Almost there!
| document.body.appendChild(textarea); | ||
| textarea.focus(); | ||
| }); | ||
| await page.keyboard.type('Hello{Enter}World', { namedKeys: true }); |
There was a problem hiding this comment.
Let's type the following: He{{ll}o{Enter}Wor{ld and see that it produces He{ll}o\nWor{ld
There was a problem hiding this comment.
Done. Updated the test to type He{{ll}o{Enter}Wor{ld and verify it produces He{ll}o\nWor{ld, covering escaped braces, unmatched braces, and named keys.
| } else { | ||
| for (const token of parseNamedKeys(text, !!options?.namedKeys)) { | ||
| if (token.type === 'key') { | ||
| if (delay) |
There was a problem hiding this comment.
Why an extra delay here when we pass it to this.press() in the next line? Looks different from how it was before.
There was a problem hiding this comment.
Done. Removed the extra progress.wait(delay) before this.press(). Now the named key branch matches the original behavior for layout characters, where press() handles delay internally.
- Remove extraneous progress.wait(delay) before this.press() in the
named key branch, matching the original behavior for layout characters
where press() handles delay internally.
- Update namedKeys test to use edge-case input as suggested: escaped
braces ({{), unmatched }, named key ({Enter}), and unmatched {.
- Regenerate types to sync with doc wording updates.
Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>Test results for "MCP"2 failed 7097 passed, 1104 skipped Merge workflow run. |
Test results for "tests 1"3 flaky41925 passed, 850 skipped Merge workflow run. |
Dmitry Gozman (dgozman)
left a comment
There was a problem hiding this comment.
Looks great, merging in. Thank you for the PR!
7f13406
into
microsoft:mainUh oh!
There was an error while loading. Please reload this page.
Summary
namedKeysoption toLocator.pressSequentiallyFixes#40740