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

fix: make clicks on type('{enter}') composed #26395

Merged
merged 11 commits into from
Apr 10, 2023

Conversation

kgroat
Copy link
Contributor

@kgroat kgroat commented Apr 2, 2023

Additional details

When typing {enter} in a shadow root, the click event it generates doesn't propagate through shadow roots. This PR fixes that by setting composed: true. See https://developer.mozilla.org/en-US/docs/Web/API/Event/composed

Steps to test

Clone https://github.com/kgroat/cypress-enter-repro and run the tests.

How has the user experience changed?

The UX has remain unchanged, just that the click event propagates through shadow roots.

PR Tasks

@kgroat kgroat changed the title Make clicks on type('{enter}') composed fix: make clicks on type('{enter}') composed Apr 2, 2023
@kgroat kgroat force-pushed the issue-26392-make-click-composed branch from 478da43 to 70af6f1 Compare April 2, 2023 14:07
@kgroat kgroat marked this pull request as ready for review April 4, 2023 15:32
@kgroat
Copy link
Contributor Author

kgroat commented Apr 4, 2023

I noticed that the test file I updated (packages/driver/cypress/e2e/commands/actions/type_events_spec.js) doesn't have .cy. in it, so it isn't getting picked up by cypress & run locally. Is this something I should fix here, or is that something I should just leave alone?

@cypress
Copy link

cypress bot commented Apr 4, 2023

1 failed and 29 flaky tests on run #45243 ↗︎

1 26908 1281 0 Flakiness 29

Details:

Merge branch 'develop' into issue-26392-make-click-composed
Project: cypress Commit: 74937f146d
Status: Failed Duration: 19:32 💡
Started: Apr 4, 2023 3:42 PM Ended: Apr 4, 2023 4:02 PM
Failed  cypress/e2e/e2e/origin/commands/waiting.cy.ts • 1 failed test • 5x-driver-electron

View Output Video

Test Artifacts
cy.origin waiting > alias > waits for the route alias to have a response Output Video
Flakiness  create-from-component.cy.ts • 1 flaky test • app-e2e

View Output Video

Test Artifacts
... > runs generated spec Output Screenshots Video
Flakiness  specs_list_latest_runs.cy.ts • 1 flaky test • app-e2e

View Output Video

Test Artifacts
App/Cloud Integration - Latest runs and Average duration > when no runs are recorded > shows placeholders for all visible specs Output Screenshots Video
Flakiness  cypress-in-cypress-component.cy.ts • 1 flaky test • app-e2e

View Output Video

Test Artifacts
Cypress In Cypress CT > default config > redirects to the specs list with error if a spec is not found Output Screenshots Video
Flakiness  cypress-origin-communicator.cy.ts • 1 flaky test • app-e2e

View Output Video

Test Artifacts
Cypress In Cypress Origin Communicator > cy.origin passivity with app interactions > passes upon test reload mid test execution Output Screenshots Video
Flakiness  commands/net_stubbing.cy.ts • 1 flaky test • 5x-driver-firefox

View Output Video

Test Artifacts
network stubbing > intercepting request > can delay and throttle a StaticResponse Output

The first 5 flaky specs are shown, see all 17 specs in Cypress Cloud.

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@marktnoonan
Copy link
Contributor

@kgroat good eye! Please update the filename to include cy and let's see what happens, this seems like it might have been an oversight

@kgroat kgroat changed the title fix: make clicks on type('{enter}') composed fix: make clicks on typing enter composed Apr 4, 2023
@kgroat kgroat changed the title fix: make clicks on typing enter composed fix: make clicks on type('{enter}') composed Apr 4, 2023
@kgroat
Copy link
Contributor Author

kgroat commented Apr 4, 2023

I don't understand what this error means in the Semantic Pull Request job:

Error: Expected line number 17 to be a line break

I've tried modifying the title & description of the PR but nothing makes it go away.

@astone123
Copy link
Contributor

@kgroat sorry about that, we had merged a change in that had an incorrect changelog. I just merged a commit into develop to fix it and just synced the PR so we should be good on that

@kgroat
Copy link
Contributor Author

kgroat commented Apr 5, 2023

It looks like renaming commands/actions/type_events.cy.js is causing some failures in CI. Should I rename it back & move my test into a different file, or what is the path forward?

@astone123
Copy link
Contributor

@kgroat it looks like those tests are failing on the Webkit browser. Adding this spec back might have uncovered an issue. In the interest of moving this forward, I'm going to try to exclude this specific spec from running on Webkit and make a follow up issue to fix that

@astone123 astone123 force-pushed the issue-26392-make-click-composed branch from 932b4e0 to 8c27f98 Compare April 6, 2023 16:04
@astone123 astone123 force-pushed the issue-26392-make-click-composed branch from 8c27f98 to a457704 Compare April 6, 2023 16:08
@astone123 astone123 requested a review from chrisbreiding April 6, 2023 16:10
@astone123
Copy link
Contributor

I'm trying to figure out why component tests are failing on this branch. That's the only thing keeping this from merging

cli/CHANGELOG.md Outdated Show resolved Hide resolved
@astone123
Copy link
Contributor

The failures on run-app-component-tests-chrome are unrelated to this PR

@nagash77 nagash77 merged commit 92542ea into cypress-io:develop Apr 10, 2023
astone123 added a commit to kgroat/cypress that referenced this pull request Apr 19, 2023
* fix: make clicks on type('{enter}') composed

Co-authored-by: Mike Plummer <[email protected]>

* disable failing type_events driver tests from running in Webkit

* make sure done is only called once

* use test configuration to disable on webkit

* fix changelog

* Update cli/CHANGELOG.md

Co-authored-by: Emily Rohrbough <[email protected]>

---------

Co-authored-by: Adam Stone-Lord <[email protected]>
Co-authored-by: Mike Plummer <[email protected]>
Co-authored-by: Emily Rohrbough <[email protected]>
Co-authored-by: Ben M <[email protected]>
tgriesser added a commit that referenced this pull request Apr 25, 2023
* feat/protocol: (45 commits)
  chore: adding support for url:changed (#26519)
  chore: adding viewport:changed to protocol (#26508)
  chore: Reduce dependencies and binary size, add circle ci detector (#26522)
  chore: 12.10.0 release (#26517)
  test: fix flaky tests (#26505)
  chore: Check project dependencies for CT compatibility (#26497)
  chore: update vm2 to 3.9.16 (#26489)
  chore: enable builds on feat/protocol branch (#26506)
  chore: [skip ci] update to labels looked at by stalebot (#26496)
  chore: connecting to electron browser (#26471)
  chore: [skip ci] turning on stale bot (#26488)
  chore: fix issue with logs without wallClockUpdatedAt (#26473)
  Update triage_add_to_project.yml
  chore: Update Chrome (stable) to 112.0.5615.49 and Chrome (beta) to 113.0.5672.24 (#26434)
  feat: display framework definition errors (#26183)
  fix: correctly resolve dependencies for CT onboarding when using Yarn Plug n Play (#26452)
  fix: Subscribe to framework detection changes in wizard (#26437)
  fix: make clicks on type('{enter}') composed (#26395)
  chore: update add-to-project workflow (#26439)
  chore: Pass telemetry resources from the node process to the browser (#26468)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants