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

feat: Implement testing type switch promos #26894

Merged
merged 42 commits into from
Jun 6, 2023

Conversation

mike-plummer
Copy link
Contributor

@mike-plummer mike-plummer commented May 31, 2023

Additional details

A new testing type switcher is being added to the Specs Explorer. It has the following behaviors:

If other testing type is configured

  • no "question mark" icon is displayed
  • Clicking the other testing type immediately switches the active testing type (closes and reopens browser).

If other testing type is not configured

  • A question mark icon is displayed
  • Clicking the other testing type hides the specs list and shows a Promo with informational content about that testing type.
  • Promo will:
    • Record an anon event on display
    • Provide a "Quick setup" button that closes browser and sends user to setup flow for other testing type. Clicking button records a machine-collect event
    • Provides docs and framework links with appropriate UTM params

Figma

Note: The following discrepancies with the Figma have been surfaced with design and approved:

  1. Different style of Question Mark icon in switcher
  2. Switcher is shorter (complies with DS styling)
  3. No "skeleton" rows are displayed behind promo
  4. There's some extra height from the SpecsList virtualist which causes the promo to center a bit low - going to see how Adam's changes to SpecsList styling impact this

Steps to test

  1. Create a project with CT and E2E set up
  2. Open project using this branch, observe switcher displays with no question mark. Clicking each testing type should move Cypress into target testing type.
  3. Close project. Edit config file and comment out "component" entry. Reopen project into E2E
  4. Switcher should display question mark on CT mode. Click CT button and observe CT promo is displayed. Verify links include appropriate UTM params. Click Quick Setup and verify user is sent to launchpad setup flow for CT.
  5. Close project. Edit config file - uncomment "component" and comment out "e2e" config. Reopen project into CT.
  6. Switcher should display question mark on E2E mode. Click E2E button and observe E2E promo is displayed. Verify links include appropriate UTM params. Click Quick Setup and verify user is sent to E2E mode.

How has the user experience changed?

Both testing types configured
Screenshot 2023-05-31 at 1 26 09 PM

CT not configured
Screenshot 2023-05-31 at 1 26 56 PM

E2E not configured
Screenshot 2023-05-31 at 1 27 53 PM

PR Tasks

@mike-plummer mike-plummer requested a review from a team May 31, 2023 20:58
@mike-plummer mike-plummer marked this pull request as ready for review May 31, 2023 20:58
@cypress
Copy link

cypress bot commented May 31, 2023

5 flaky tests on run #47125 ↗︎

0 5193 77 0 Flakiness 5

Details:

text formatting
Project: cypress Commit: 82d0aceaad
Status: Passed Duration: 13:25 💡
Started: Jun 6, 2023 12:27 AM Ended: Jun 6, 2023 12:41 AM
Flakiness  commands/net_stubbing.cy.ts • 1 flaky test • 5x-driver-electron

View Output Video

Test Artifacts
network stubbing > intercepting request > can delay and throttle a StaticResponse Output Video
Flakiness  e2e/origin/navigation.cy.ts • 1 flaky test • 5x-driver-electron

View Output Video

Test Artifacts
delayed navigation > errors > redirects to an unexpected cross-origin and calls another command in the cy.origin command Output Video
Flakiness  cypress/cypress.cy.js • 3 flaky tests • 5x-driver-electron

View Output Video

Test Artifacts
... > correctly returns currentRetry Output Video
... > correctly returns currentRetry Output Video
... > correctly returns currentRetry Output Video

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

@astone123
Copy link
Contributor

A couple of questions before I dive into the code:

  1. Should the testing type switcher persist state if the user navigates to a different page and then back to the specs list? Right now if you navigate to a testing type and see the promo, then navigate away and back to it, the switcher goes back to the current testing type. This makes intuitive sense to me, I just want to make sure that was intentional.
  2. There's some weirdness in the UI:

After switching testing types once I saw it like this (this was immediately after I used the switcher to set up e2e testing, not sure if relevant):
Screenshot 2023-06-01 at 8 27 46 AM

Also, it looks like every time we switch testing types using the switcher, the selected UI testing type shows up during the loading state for the browser. For example (can't believe I screenshotted this fast enough)
Screenshot 2023-06-01 at 8 30 12 AM
This looks kind of strange to me. Anything we can do about that?

@mike-plummer
Copy link
Contributor Author

@astone123

  1. Correct, there are no state persistence requirements for the switcher, so the expectation at present is that it will always default to the "active" testing type after a refresh or navigation
  2. Weird. I'll take a look at the horizontal overlap issue, might be conflicting with the grid column widths we're using in the header
  3. I saw that yesterday while demoing with Mark - I think there may be a hard z-index override on that component. I'll take a look

Copy link
Contributor

@astone123 astone123 left a comment

Choose a reason for hiding this comment

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

Just a few comments. Also I'm curious why the snapshot cache files were updated?

cli/CHANGELOG.md Outdated Show resolved Hide resolved
@mike-plummer
Copy link
Contributor Author

mike-plummer commented Jun 1, 2023

Just a few comments. Also I'm curious why the snapshot cache files were updated?

@astone123 kitchensink and unit tests are failing due to weird errors related to require & package.json. I ran the snapshot update task just to make sure it wasn't a snapshot mismatch since there are updates to the yarn.lock in this PR

@astone123
Copy link
Contributor

Just tested again. I'm not seeing the issues that I saw before. One more thing, should the bottom of the promo have this much padding? Looks a little off to me

Screenshot 2023-06-01 at 11 56 59 AM

* Fix issue with yarn.lock
* Fix extra padding at bottom of promo
* Add tests for utm params
* Add test for switching testing type when both configured
* Fix changelog version
@mike-plummer
Copy link
Contributor Author

Just tested again. I'm not seeing the issues that I saw before. One more thing, should the bottom of the promo have this much padding? Looks a little off to me

@astone123 Removed extra padding in latest commit

@marktnoonan
Copy link
Contributor

Still testing, here's an edge case I found - if I remove the component testing configuration while CT is open, we don't seem to handle that, and the UI gets to an undefined state (no "project/branch" item in the top left of the nav).

I guess ideally we'd put you back at the launchpad to choose your testing type again and either you open e2e or set up CT.

I haven't tested the current behavior, maybe this PR doesn't have anything to do with it.

Screenshot 2023-06-01 at 4 02 49 PM

@astone123
Copy link
Contributor

@mike-plummer One more minor thing I'm seeing - when I launch a project that has e2e configured but not CT, when the app loads I see the CT promo flash on screen very quickly. Probably not a huge deal, but I imagine we could prevent that from happening for a smoother experience.

For example
https://www.loom.com/share/f40652ad01994d458c50bc52e880d7fc

@marktnoonan
Copy link
Contributor

Overall this is looking really great. One question on the size of the full-width version, I couldn't put my finger on this yesterday when we talked about the button size.

In Figma the full-width variant is 858px. This produces less text wrapping and a good amount of space between the title and the button:

Screenshot 2023-06-01 at 4 42 58 PM

It looks like our promo component doesn't get bigger than 480px. Is this known/intentional at this point, based on other existing uses of PromoCard?

@astone123
Copy link
Contributor

astone123 commented Jun 5, 2023

@marktnoonan looks good overall, but I'm still seeing this behavior that I mentioned in this comment. Not a huge issue but it would be nice if it was an easy change

Edit: Actually what I'm seeing now is that the e2e promo is flashing on screen (which is the testing type that is configured)

@astone123 astone123 self-requested a review June 5, 2023 20:13
Copy link
Contributor

@astone123 astone123 left a comment

Choose a reason for hiding this comment

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

I just tested again, looks good!

@astone123
Copy link
Contributor

@lmiller1990
Copy link
Contributor

I think I still saw the flashing banner bug, but I realized it was another banner. I am unsure if it was introduced here, but I suspect it was not. I'll find out.

Here's the bug recorded. It only happens sometimes, the first time you open a project?

bug.mov

@lmiller1990
Copy link
Contributor

The actual feature works great, I tested it out, looks good!

@lmiller1990
Copy link
Contributor

lmiller1990 commented Jun 6, 2023

I re-ran yarn and I can no longer reproduce the flicking bug I found on either this branch or develop. I'm not sure why it happened, but I don't think it was introduced here at any rate.

Edit, maybe I'd missed 54cb1a0 where @marktnoonan fixed the issue. It seems fine now!

@lmiller1990
Copy link
Contributor

I pushed a single commit after getting it all green that removed a superfluous snapshot (this component is captured in several others): 59df96a.

@lmiller1990 lmiller1990 merged commit 25582dd into develop Jun 6, 2023
@lmiller1990 lmiller1990 deleted the mikep/589-testing-type-switch branch June 6, 2023 02:02
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jun 7, 2023

Released in 12.14.0.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v12.14.0, please open a new issue.

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Jun 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create "Setup CT" Promo Create "Setup E2E" Promo Add Testing Type switcher to Specs Explorer
5 participants