Add {home} and {end} to cy.type() #3071

silviuaavram · 2019-01-07T15:40:33Z

PR close #2033 by implementing handlers for Home and End in function type.

I am creating a PR for Downshift library in which I am implementing keyboard handling for Home and End in the context of a Dropdown (Home highlights first option, End the last one). PR is here. I meant to write end-to-end tests as well. Since the repo uses Cypress, I found that it did not support {home} and {enter} so I found this issue in your repo and am trying to fix it.

Work is still in progress, I still mean to write UTs and E2Es. However, I appreciate early feedback on the implementation as well, since this could be my first commit in this repository.

CLAassistant · 2019-01-07T15:40:41Z

All committers have signed the CLA.

jennifer-shehane · 2019-01-07T17:18:21Z

@silviuavram Thanks for signing the CLA, unfortunately the author of the commits does not match the email address on your GitHub account. Our CLA bot gets confused when they don't match. Here are some solutions to fixing when commits are linked to the wrong user: https://help.github.com/articles/why-are-my-commits-linked-to-the-wrong-user/

kuceb · 2019-01-07T17:27:56Z

Hi @silviuavram, thanks for this PR.

You pulled the selection utils from #2436 correct? Looks pretty good, however

for home and end, we need to take into account whether focused element is an input, and if not, set scrollTop=0 or scrollHeight on the innermost scrollable el.

kuceb · 2019-01-07T17:30:21Z

packages/driver/src/cypress/keyboard.coffee

+      options.input     = false
+      options.setKey    = "{home}"
+      @ensureKey el, null, options, ->
+        $selection.moveCursorToStart(el)


~~if not an input, set scrollTop=0 on innermost scrollable el~~

kuceb · 2019-01-07T17:30:39Z

packages/driver/src/cypress/keyboard.coffee

+      options.input     = false
+      options.setKey    = "{end}"
+      @ensureKey el, null, options, ->
+        $selection.moveCursorToEnd(el)


~~if not an input, set scrollTop=el.scrollHeight` on innermost scrollable el~~

edit: we can forsake this default action, unless someone expresses desire for it

a bit confusing for me.

i know that when you use Home and End, and you are not in input/textarea, it should scroll to top/bottom of page.

however there is a similar story with Up Arrow and Down Arrow, they scroll up/down a little bit.

that being said, I don't see any scrolling stuff in their handlers here. it's the reason I did not add anything to my handlers either.

what am i missing @bkucera ?

I see what you're saying about up/down arrow not currently scrolling the page. We probably wouldn't end up fulfilling that default action, and only fulfill the default action of the {home} and {end} keys inside of text editables

However, the selection.modify API isn't available in IE11 (even though there are ways to polyfill it) so we should be hesitant to use it. @silviuavram is your app preventing default on the home key press, or do you need the default action (move to end of line)?

Resumed working on this and renamed a variable as it was used for two reasons in the same function: start. This is what happens when you are not paying attention...

Anyway, about the {home} and {end} keys, my use is to create e2e tests for a Dropdown, and the scenario tests that, with focus on Edit Text and Menu open, Home highlights the first option in the menu, while End highlights the last one.

I want to fully understand how this works in keyboard.coffee. If there is an element focused, we call, for example, $selection.moveCursorToLineStart(el), to simulate the cursor behaviour for Home/End inside an <input> or <textarea>? If no focused element, what happens then? Are the Home/End default behaviours happening anyway? (scrolling to the beginning/end of the page, if no input element is focused).

Or maybe point me to some documentation maybe that will be easier. And also since I mentioned pointing at stuff, I cannot find where to add unit tests for my changes. Can you also help me with this, please?

Thank you for your time! @bkucera

@silviuavram yes, inside a text-editable (input, text-area, or contentedtiable), we send synthetic events and we move the cursor, if of course the event was not canceled on keydown (taken care of for you)

if it's not a text-editable, we should send synthetic events but don't move the cursor, and just skip over the page scrolling for now. This would happen if you used cy.get('body').type(...) for example.

Tests should go in packages/driver/test/cypress/integration/commands/actions/type_spec.coffee
and you could add a new describe block testing the functionality of using permutations of cy.get('input/textarea/[contenteditable]/body').type({home}/{end})

I started to add some unit tests. Will continue tomorrow. You can take a fast look to see if they're on the right track.

I also refactored the _moveCursorToLineStartOrEnd method and used only selection.modify to move that cursor, for everything (contenteditable, textarea, input). I don't think there is any reason to do some \n search and move the cursor with setSelectionRange after doing some math magic to locate the line start/end, right?

Hopefully if this is on the right track we can merge it soon. Thanks very much for the help! @bkucera

@silviuavram sure, modern browsers support selection.modify so that's fine.

I believe you are referring to integration tests, not unit?

@bkucera great to hear that.

As for the tests, I think I've added kind of unit tests. You can check now in type_spec.coffee where I added for {home} and {end}. But please let me know if I need to add more and maybe point me to location. The test codebase is pretty massive.

Thank you very much for assistance!

silviuaavram · 2019-01-07T17:49:10Z

Thanks everyone for the quick feedback, much appreciated!

Will fix the issues you pointed out tomorrow and will also write the tests for the changes. Will come back to you afterwards!

@bkucera I am not sure what you mean by your PR, maybe I am missing some context :) . The branch is branched out from latest develop, did not merge anything else in it. Let me know if I need any more information related to this, thanks!

packages/driver/src/dom/selection.js

**What**: This PR aims to improve kb navigation inside the listbox by adding support for `Home` and `End`. These keys should highlight first / last element, if the listbox is visible. **Why**: Improves the component by adding another ARIA specification regarding kb navigation. **How**: Added event listeners for keydown on both `Home` and `End`, and added handlers for each, in which I've used variations for the `setHighlightedIndex` function. Also added event names for both kets. Also added UTs for these changes, and planning to add E2E Cypress tests too. These are already written, however Cypress does not support Home and End at the moment. I've created a PR for implementing this in their repo [here](cypress-io/cypress#3071). Those tests are currently commented. **Checklist**: - [ ] Documentation - [ ] Tests - [ ] Ready to be merged  - [ ] Added myself to contributors table

silviuaavram · 2019-01-23T17:08:22Z

@bkucera please see my last comment. I've added unit tests and I think we're good to go after you also take another look.

Thanks!

kuceb · 2019-01-23T21:29:58Z

@silviuavram LGTM. @brian-mann can this go in 3.2?

We will need to update the docs for cy.type as well

jennifer-shehane · 2019-01-24T08:01:28Z

Opened an issue for adding docs for {home} and {end} to supported sequences here: cypress-io/cypress-documentation#1344

silviuaavram · 2019-03-13T21:18:21Z

What happened to this one? Are these changes added already or something else? :)

jennifer-shehane · 2019-03-14T03:22:53Z

Oh no, I deleted the branch it was set to merge into, in an attempt to rename the branch. I didn't realize this would close the PRs against the deleted branch. What a mess. 😝 I will fix this.

implemented fix draft

48d1abd

silviuaavram mentioned this pull request Jan 7, 2019

Handle End and Home Keys in list navigation downshift-js/downshift#646

Merged

4 tasks

jennifer-shehane requested a review from kuceb January 7, 2019 17:18

kuceb suggested changes Jan 7, 2019

View reviewed changes

kuceb reviewed Jan 7, 2019

View reviewed changes

packages/driver/src/dom/selection.js Show resolved Hide resolved

removed an extra comment

852a9ca

kuceb mentioned this pull request Jan 10, 2019

Add support for {home} and {end} special character sequences #2033

Closed

Silviu Avram added 5 commits January 15, 2019 13:59

fixed two colliding variable names

a3e9542

actual behaviour for home/end is begin/end of line, not paragraph

79f6d02

refactored the selection method to use only selection.modify

2507280

added some unit tests for {home}

dd5f421

added more tests for home and end

009c643

jennifer-shehane requested a review from kuceb January 23, 2019 17:09

disclaimer for selection.modify API

be36ab7

kuceb approved these changes Jan 23, 2019

View reviewed changes

jennifer-shehane requested a review from brian-mann January 24, 2019 07:56

jennifer-shehane mentioned this pull request Jan 24, 2019

Add {home} and {end} to supported sequences in cy.type() cypress-io/cypress-documentation#1344

Closed

jennifer-shehane changed the base branch from develop to v3.2.0 January 24, 2019 18:29

brian-mann changed the title ~~Fix Issue 2033~~ Add {home} and {end} to cy.type() Jan 24, 2019

jennifer-shehane closed this Mar 13, 2019

jennifer-shehane changed the base branch from v3.2.0 to develop March 14, 2019 03:23

jennifer-shehane reopened this Mar 14, 2019

brian-mann merged commit 2122320 into cypress-io:develop Mar 15, 2019

jennifer-shehane mentioned this pull request Mar 29, 2019

cy.type('{insert}') is not supported #3845

Closed

clarmso mentioned this pull request Apr 5, 2019

#3845 Attempt to add insert, page up and page down keys #3892

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add {home} and {end} to cy.type() #3071

Add {home} and {end} to cy.type() #3071

silviuaavram commented Jan 7, 2019 •

edited by jennifer-shehane

Loading

CLAassistant commented Jan 7, 2019 •

edited

Loading

jennifer-shehane commented Jan 7, 2019

kuceb commented Jan 7, 2019 •

edited

Loading

kuceb Jan 7, 2019 •

edited

Loading

kuceb Jan 7, 2019 •

edited

Loading

silviuaavram Jan 8, 2019 •

edited

Loading

kuceb Jan 9, 2019 •

edited

Loading

silviuaavram Jan 15, 2019

kuceb Jan 15, 2019 •

edited

Loading

silviuaavram Jan 17, 2019

kuceb Jan 17, 2019

silviuaavram Jan 17, 2019

silviuaavram commented Jan 7, 2019

silviuaavram commented Jan 23, 2019

kuceb commented Jan 23, 2019 •

edited

Loading

jennifer-shehane commented Jan 24, 2019

silviuaavram commented Mar 13, 2019

jennifer-shehane commented Mar 14, 2019

Add {home} and {end} to cy.type() #3071

Add {home} and {end} to cy.type() #3071

Conversation

silviuaavram commented Jan 7, 2019 • edited by jennifer-shehane Loading

CLAassistant commented Jan 7, 2019 • edited Loading

jennifer-shehane commented Jan 7, 2019

kuceb commented Jan 7, 2019 • edited Loading

kuceb Jan 7, 2019 • edited Loading

Choose a reason for hiding this comment

kuceb Jan 7, 2019 • edited Loading

Choose a reason for hiding this comment

silviuaavram Jan 8, 2019 • edited Loading

Choose a reason for hiding this comment

kuceb Jan 9, 2019 • edited Loading

Choose a reason for hiding this comment

silviuaavram Jan 15, 2019

Choose a reason for hiding this comment

kuceb Jan 15, 2019 • edited Loading

Choose a reason for hiding this comment

silviuaavram Jan 17, 2019

Choose a reason for hiding this comment

kuceb Jan 17, 2019

Choose a reason for hiding this comment

silviuaavram Jan 17, 2019

Choose a reason for hiding this comment

silviuaavram commented Jan 7, 2019

silviuaavram commented Jan 23, 2019

kuceb commented Jan 23, 2019 • edited Loading

jennifer-shehane commented Jan 24, 2019

silviuaavram commented Mar 13, 2019

jennifer-shehane commented Mar 14, 2019

silviuaavram commented Jan 7, 2019 •

edited by jennifer-shehane

Loading

CLAassistant commented Jan 7, 2019 •

edited

Loading

kuceb commented Jan 7, 2019 •

edited

Loading

kuceb Jan 7, 2019 •

edited

Loading

kuceb Jan 7, 2019 •

edited

Loading

silviuaavram Jan 8, 2019 •

edited

Loading

kuceb Jan 9, 2019 •

edited

Loading

kuceb Jan 15, 2019 •

edited

Loading

kuceb commented Jan 23, 2019 •

edited

Loading