fix type when previous selection in input in some cases #5809

kuceb · 2019-11-27T17:57:00Z

Closes 3.5.0-3.6.1 regression - cy.type() may skip a character on a autofocussing textbox #5703

User facing changelog

fixes issue with .type not respecting previous selection in non-selectionRange input element (email, number) (only way to change selection is with .select())
fixes issue with .type not respecting previous selection if current value of the input is same as key

Additional details

code from firefox branch was pulled in, causing the large amount of files changed. This will help reduce changes needed for firefox as well

How has the user experience changed?

when attempting to use `.type` on a number or email input that has a text selection (or is selected in a focus handler):

before (incorrect): text is appended to the current selection
after (correct): text replaces current selection

when attempting to use `.type` on an input that has a text selection (or is selected in a focus handler) of 1 character equal to the key typed:

before (incorrect): ignores current key, next character replaces selection
after (correct): text replaces current selection, next character is typed after

PR Tasks

Have tests been added/updated?
Has the original issue been tagged with a release in ZenHub?

cypress-bot · 2019-11-27T17:57:02Z

Thanks for the contribution! Below are some guidelines Cypress uses when doing PR reviews.

Please write [WIP] in the title of your Pull Request if your PR is not ready for review - someone will review your PR as soon as the [WIP] is removed.
Please familiarize yourself with the PR Review Checklist and feel free to make updates on your PR based on these guidelines.

PR Review Checklist

If any of the following requirements can't be met, leave a comment in the review selecting 'Request changes', otherwise 'Approve'.

User Experience

The feature/bugfix is self-documenting from within the product.
The change provides the end user with a way to fix their problem (no dead ends).

Functionality

The code works and performs its intended function with the correct logic.
Performance has been factored in (for example, the code cleans up after itself to not cause memory leaks).
The code guards against edge cases and invalid input and has tests to cover it.

Maintainability

The code is readable (too many nested 'if's are a bad sign).
Names used for variables, methods, etc, clearly describe their function.
The code is easy to understood and there are relevant comments explaining.
New algorithms are documented in the code with link(s) to external docs (flowcharts, w3c, chrome, firefox).
There are comments containing link(s) to the addressed issue (in tests and code).

Quality

The change does not reimplement code.
There's not a module from the ecosystem that should be used instead.
There is no redundant or duplicate code.
There are no irrelevant comments left in the code.
Tests are testing the code’s intended functionality in the best way possible.

Internal

The original issue has been tagged with a release in ZenHub.

cypress · 2019-11-27T18:18:21Z

Test summary

3520 • 0 • 45 • 0

Run details


Project	cypress
Status	Passed
Commit	`1859d5c`
Started	Nov 27, 2019 8:15 PM
Ended	Nov 27, 2019 8:19 PM
Duration	04:17 💡
OS	Linux Debian - 9.8
Browser	Multiple

View run in Cypress Dashboard ➡️

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

brian-mann · 2019-11-27T19:44:13Z

packages/driver/src/cy/keyboard.ts

+      const curVal = $elements.getNativeProp(el, 'value')
+      const bounds = $selection.getSelectionBounds(el)
+
+      const potentialValue = $selection.insertSubstring(curVal + needsValue, key.text, [bounds.start + needsValueLength, bounds.end + needsValueLength])


i feel like this needs an explanation... its so opaque why this is necessary

packages/driver/src/cy/keyboard.ts

brian-mann · 2019-11-27T19:52:50Z

packages/driver/test/cypress/integration/commands/actions/type_spec.js

+          e.currentTarget.select()
+        })
+
+        // input[0].setSelectionRange(0,0)


brian-mann · 2019-11-27T19:53:24Z

packages/driver/src/dom/selection.ts

  return false
 }

 const getCaretPosition = function (el) {
  const bounds = getSelectionBounds(el)

-  if (bounds.start == null) {
+  if ((bounds.start == null)) {


brian-mann · 2019-11-27T19:54:21Z

packages/driver/src/dom/selection.ts

-        start: 0,
-        end: 0,
+        start: null,
+        end: null,


switch back to switch (true)

brian-mann

minor changes per my comments

brian-mann · 2019-11-27T19:57:14Z

packages/driver/src/dom/elements.ts

+const isAttrType = function (el: HTMLInputElement, type: string) {
+  const elType = (el.getAttribute('type') || '').toLowerCase()
+
+  if (elType === type) {
+    return true
+  }
+
+  return false
+}


#1359 (comment)

brian-mann · 2019-11-27T20:07:55Z

packages/driver/test/cypress/integration/commands/actions/type_spec.js

+        .should('have.value', '10')
+      })
+
+      it('overwrites text when selectAll in focus handler in date', () => {


email not date

This reverts commit bfb9d83.

) * fix type when previous selection in input * cleanup * cleanup more * more cleanup * more more cleanup final

…ess-io#5809)" This reverts commit bfb9d83.

fix type when previous selection in input

7dada54

kuceb requested review from brian-mann and jennifer-shehane November 27, 2019 17:57

cleanup

4aeb67b

brian-mann reviewed Nov 27, 2019

View reviewed changes

packages/driver/src/cy/keyboard.ts Show resolved Hide resolved

brian-mann reviewed Nov 27, 2019

View reviewed changes

packages/driver/src/dom/selection.ts Outdated

start: 0,

end: 0,

start: null,

end: null,

Copy link

Member

brian-mann Nov 27, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

switch back to switch (true)

brian-mann requested changes Nov 27, 2019

View reviewed changes

cleanup more

12d6e4b

brian-mann reviewed Nov 27, 2019

View reviewed changes

more cleanup

74acb11

brian-mann reviewed Nov 27, 2019

View reviewed changes

more more cleanup final

1859d5c

brian-mann approved these changes Nov 27, 2019

View reviewed changes

flotwig merged commit bfb9d83 into develop Nov 27, 2019

cypress-bot bot mentioned this pull request Nov 27, 2019

3.5.0-3.6.1 regression - cy.type() may skip a character on a autofocussing textbox #5703

Closed

flotwig added a commit that referenced this pull request Nov 27, 2019

Revert "fix type when previous selection in input in some cases (#5809)"

0ddedfa

This reverts commit bfb9d83.

flotwig mentioned this pull request Nov 27, 2019

Revert "fix type when previous selection in input in some cases" #5812

Merged

avallete pushed a commit to avallete/cypress that referenced this pull request Nov 28, 2019

fix type when previous selection in input in some cases (cypress-io#5809

9a962a5

) * fix type when previous selection in input * cleanup * cleanup more * more cleanup * more more cleanup final

avallete pushed a commit to avallete/cypress that referenced this pull request Nov 28, 2019

Revert "fix type when previous selection in input in some cases (cypr…

f9b5bc4

…ess-io#5809)" This reverts commit bfb9d83.

kuceb mentioned this pull request Dec 3, 2019

try 2: fix type when previous selection in input in some cases #5854

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix type when previous selection in input in some cases #5809

fix type when previous selection in input in some cases #5809

kuceb commented Nov 27, 2019 •

edited

cypress-bot bot commented Nov 27, 2019

cypress bot commented Nov 27, 2019 •

edited

brian-mann Nov 27, 2019

brian-mann Nov 27, 2019

brian-mann Nov 27, 2019

brian-mann Nov 27, 2019

brian-mann left a comment

brian-mann Nov 27, 2019

brian-mann Nov 27, 2019

fix type when previous selection in input in some cases #5809

fix type when previous selection in input in some cases #5809

Conversation

kuceb commented Nov 27, 2019 • edited

User facing changelog

Additional details

How has the user experience changed?

when attempting to use .type on a number or email input that has a text selection (or is selected in a focus handler):

when attempting to use .type on an input that has a text selection (or is selected in a focus handler) of 1 character equal to the key typed:

PR Tasks

cypress-bot bot commented Nov 27, 2019

PR Review Checklist

User Experience

Functionality

Maintainability

Quality

Internal

cypress bot commented Nov 27, 2019 • edited

Test summary

Run details

brian-mann Nov 27, 2019

Choose a reason for hiding this comment

brian-mann Nov 27, 2019

Choose a reason for hiding this comment

brian-mann Nov 27, 2019

Choose a reason for hiding this comment

brian-mann Nov 27, 2019

Choose a reason for hiding this comment

brian-mann left a comment

Choose a reason for hiding this comment

brian-mann Nov 27, 2019

Choose a reason for hiding this comment

brian-mann Nov 27, 2019

Choose a reason for hiding this comment

kuceb commented Nov 27, 2019 •

edited

when attempting to use `.type` on a number or email input that has a text selection (or is selected in a focus handler):

when attempting to use `.type` on an input that has a text selection (or is selected in a focus handler) of 1 character equal to the key typed:

cypress bot commented Nov 27, 2019 •

edited