Skip to content
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

selectall / unselect regressions in 3.5.0, 3.6 #5622

Closed
znarf opened this issue Nov 7, 2019 · 10 comments · Fixed by #5637 or #5696 · May be fixed by ngChile/ngx-devkit-cypress-builder#20 or qsays/grafana#1
Closed

selectall / unselect regressions in 3.5.0, 3.6 #5622

znarf opened this issue Nov 7, 2019 · 10 comments · Fixed by #5637 or #5696 · May be fixed by ngChile/ngx-devkit-cypress-builder#20 or qsays/grafana#1
Labels
pkg/driver This is due to an issue in the packages/driver directory type: regression A bug that didn't appear until a specific Cy version release v3.5.0 🐛 Issue present since 3.5.0

Comments

@znarf
Copy link

znarf commented Nov 7, 2019

Some regressions were introduced in 3.5 in type() behavior when interacting with selection (selectall / unselect).

Current behavior:

The issue happens in a rich text editor where we are making a text bold by triggering:

  • {selectall} to select the typed text
  • {ctrl}B one time to start
  • unselecting by triggering {rightarrow}.
  • {ctrl}B a second time to stop

It seems:

  • the content is not unselected by triggering {rightarrow}.
  • the {ctrl} is ignored, only B is triggered

Steps to reproduce: (app code and test code)

In a rich text editor supporting this command.

  • Type: Hello{selectall}{ctrl}B{rightarrow}{ctrl}B world!
  • Desired behavior < 3.5: Hello World !
  • Current behavior >= 3.5: B World!

Work in progress, currently trying to generate a minimum reproduction.

Sources

Versions

Introduced in Cypress 3.5, still happening in 3.6. Linux / Mac OS.

@kuceb
Copy link
Contributor

kuceb commented Nov 7, 2019

Thanks @znarf , I inadvertently introduced a breaking change with 3.5.0, with the decision to not insert text if a non-shift modifier key is held down (what the browser does).

The problem is the cy.type API is a little obtuse and, for example, the ctrl key is not just held down for the next character, but for the entire rest of the typing sequence.

I now realize that this breaks a lot of tests, since users might not write the type string wanting/knowing the modifiers are held down for the entire rest of the type.

I'll open up a PR to revert to pre 3.5.0 behavior

@kuceb kuceb added the type: regression A bug that didn't appear until a specific Cy version release label Nov 7, 2019
@jennifer-shehane jennifer-shehane added the pkg/driver This is due to an issue in the packages/driver directory label Nov 7, 2019
@jennifer-shehane jennifer-shehane changed the title selectall / unselect regressions in 3.5, 3.6 selectall / unselect regressions in 3.5.0, 3.6 Nov 7, 2019
@cypress-bot cypress-bot bot added stage: needs review The PR code is done & tested, needs review and removed stage: work in progress labels Nov 7, 2019
@cypress-bot cypress-bot bot added stage: pending release and removed stage: needs review The PR code is done & tested, needs review labels Nov 7, 2019
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Nov 7, 2019

The code for this is done in cypress-io/cypress#5637, but has yet to be released.
We'll update this issue and reference the changelog when it's released.

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Nov 8, 2019

Released in 3.6.1.

@znarf
Copy link
Author

znarf commented Nov 9, 2019

@bkucera It's still failing but the behavior is different.

CI run: https://circleci.com/gh/opencollective/opencollective-frontend/55853

  • Type: Hello{selectall}{ctrl}B{rightarrow}{ctrl}B world!
  • Desired behavior < 3.5: Hello World !
  • Older behavior >= 3.5 and < 3.6.1: B World !
  • New behavior >= 3.6.1: HelloB World !

Looks like {ctrl}B is triggered as B. Reopen?

@kuceb
Copy link
Contributor

kuceb commented Nov 11, 2019

apologies @znarf, Would you happen to know if you're using .preventDefault on the 'B' to prevent it from typing? I am not sure why the B would not be typed prior to 3.5.0 unless that is the case, but I'll check it out.

It would help if you could screenshot the cy.type events table that gets logged to the browser console when you click the command log. Could you provide a screenshot for the event table in <3.5.0 ?

@kuceb kuceb reopened this Nov 11, 2019
@Betree
Copy link

Betree commented Nov 13, 2019

Hi @bkucera, I'm working with @znarf on Open Collective.

We don't preventDefault in our code, but the library we're using - react-quill - may.

Here's a screenshot of the logs in console:

  • With 3.4.1

image

  • With 3.6.1

image


I've been able to make it work on 3.6.1 by splitting in multiple cy.type:

    cy.get('[data-cy="HTMLEditor"] .ql-editor').type('Hello');
    cy.get('[data-cy="HTMLEditor"] .ql-editor').type('{selectall}');
    cy.get('[data-cy="HTMLEditor"] .ql-editor').type('{ctrl}B');
    cy.get('[data-cy="HTMLEditor"] .ql-editor').type('{rightarrow}');
    cy.get('[data-cy="HTMLEditor"] .ql-editor').type('{ctrl}B');
    cy.get('[data-cy="HTMLEditor"] .ql-editor').type(' world!');

@kuceb
Copy link
Contributor

kuceb commented Nov 13, 2019

@Betree so in 3.4.1 if you have two of the same modifiers in a key string, we simply ignore the second one, since we treat it as still being held down. In 3.5.0+ we send a second keydown, which is causing the bug. If you remove the second ctrl from your type string, it should fix the issue. I'll open another PR to fix this and revert to <3.5.0 behavior, but this does show how our .type API is confusing for users. (notice
in the 3.4.1 screenshot how the ctrl modifier is held down after the first press, and there is no second ctrl keydown)

We should probably implement a way to type one modifier key combination + release without having to break up into multiple type commands, something like:
cy.type('{ctrl+b}hello{ctrl+b}world')

@Betree
Copy link

Betree commented Nov 13, 2019

cy.type('{ctrl+b}hello{ctrl+b}world') looks very clear and easy to read, that' a good idea!

@cypress-bot cypress-bot bot added the stage: needs review The PR code is done & tested, needs review label Nov 13, 2019
@cypress-bot cypress-bot bot added stage: work in progress stage: needs review The PR code is done & tested, needs review and removed stage: needs review The PR code is done & tested, needs review stage: work in progress labels Nov 25, 2019
@cypress-bot cypress-bot bot added stage: pending release and removed stage: needs review The PR code is done & tested, needs review labels Nov 26, 2019
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Nov 26, 2019

The code for this is done in cypress-io/cypress#5696, but has yet to be released.
We'll update this issue and reference the changelog when it's released.

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Nov 27, 2019

Released in 3.7.0.

@jennifer-shehane jennifer-shehane added the v3.5.0 🐛 Issue present since 3.5.0 label Dec 10, 2019
@cypress-io cypress-io locked as resolved and limited conversation to collaborators Jan 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pkg/driver This is due to an issue in the packages/driver directory type: regression A bug that didn't appear until a specific Cy version release v3.5.0 🐛 Issue present since 3.5.0
Projects
None yet
4 participants