fix: Ensuring current browser is synchronized between app and launchpad #20830

tbiethman · 2022-03-29T19:07:53Z

Closes UNIFY-1395

When a new browser is selected from the app's top navigation, the current browser instance is destroyed and the new browser is opened. As part of this lifecycle, the currentBrowser field is updated, and the browserStatus field is changed ('closed' -> 'opening' -> 'open'), with each change being broadcast through the browserStatusChange subscription event.

The launchpad remains open throughout this process and should be updated by its subscription to the 'browserStatusChange` event. However, because we are not using the currentBrowser field in the subscription definition, we are not getting notified that its value has changed and thus continue to show the previous browser as selected/open.

I updated the subscription to include the currentBrowser so that any changes to browserStatus will include any relevant corresponding changes to the currentBrowser. This ensures that the currentBrowser is always synchronized properly.

I also cleaned up a few things around this logic and updated tests where necessary.

User facing changelog

Fixes an issue where changing the browser from the app does not reflect in the launchpad

Additional details

How has the user experience changed?

browser_sync.mov

PR Tasks

Have tests been added/updated?
Has the original issue (or this PR, if no issue exists) been tagged with a release in ZenHub? (user-facing changes only)
Has a PR for user-facing changes been opened in cypress-documentation?
Have API changes been updated in the type definitions?
Have new configuration options been added to the cypress.schema.json?

cypress-bot · 2022-03-29T19:07:55Z

Thanks for taking the time to open a PR!

Create a Draft Pull Request if your PR is not ready for review. Mark the PR as Ready for Review when you're ready for a Cypress team member to review the PR.
Become familiar with the Code Review Checklist for guidelines on coding standards and what needs to be done before a PR can be merged.

tbiethman · 2022-03-29T19:09:01Z

packages/app/src/runner/SpecRunnerHeaderOpenMode.vue

-  }
-}
-`
-


This mutation is no longer used.

tbiethman · 2022-03-29T19:09:44Z

packages/launchpad/src/setup/OpenBrowserList.vue

          <div
            class="font-medium pt-2 text-indigo-600 text-18px leading-28px"
-            :class="{ 'text-jade-600': browser.isSelected, 'text-gray-500': browser.disabled || !browser.isVersionSupported }"
+            :class="{ 'text-jade-600': browser.id === selectedBrowserId, 'text-gray-500': browser.disabled || !browser.isVersionSupported }"


Updated this to ensure we're always reflecting the currentBrowser value in our selections.

Could we not have used the checked slot prop? We've got so many conditionals in this template 😅

Ah good call, that works great. I'll have to do some reading on how the slot props work.

tbiethman · 2022-03-29T19:15:09Z

packages/launchpad/src/setup/OpenBrowserList.vue

+    // NOTE: The currentBrowser is set to the first detected browser
+    // found during project initialization. It should always be defined.
+    return props.gql.currentBrowser?.id
+  },


I don't think the defaulting value logic was ever executed outside of component tests, as we always default the currentBrowser to the first browser detected on the user's system, which would always at least include Electron. So I removed that bit.

I also updated the markup so that if we end up in a scenario where we don't have a currentBrowser, we do not show the action buttons and allow the user to open a browser when nothing is actually selected.

tbiethman · 2022-03-29T19:16:42Z

packages/launchpad/src/setup/OpenBrowserList.vue

+    name
    displayName
-    path
-    version


I removed fields that weren't being used anymore

cypress · 2022-03-29T19:26:32Z

Test summary

4333 • 0 • 48 • 0 • 0

Run details


Project	cypress
Status	Passed
Commit	`5d0adfe`
Started	Mar 31, 2022 3:14 PM
Ended	Mar 31, 2022 3:26 PM
Duration	12:11 💡
OS	Linux Debian - 10.10
Browser	Electron 94

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

ZachJW34

Awesome PR description, tested and it works great!

ZachJW34 · 2022-03-29T19:34:41Z

packages/launchpad/src/setup/OpenBrowserList.vue

          <div
            class="font-medium pt-2 text-indigo-600 text-18px leading-28px"
-            :class="{ 'text-jade-600': browser.isSelected, 'text-gray-500': browser.disabled || !browser.isVersionSupported }"
+            :class="{ 'text-jade-600': browser.id === selectedBrowserId, 'text-gray-500': browser.disabled || !browser.isVersionSupported }"


Could we not have used the checked slot prop? We've got so many conditionals in this template 😅

* 10.0-release: fix: add index.mjs to the published files of cli (#20884) refactor: lift indexHtmlFile up to component, add validation (#20870) fix: allow migration of pluginsFile using `env` properties (#20770) fix: viewport from CLI on CT (#20849) fix: git data source unit test failure (#20875) fix: Ensuring current browser is synchronized between app and launchpad (#20830) Fix missed await on merge conflict resolution test(unification): move record keys to contexts (#20860) test: move record keys to contexts (#20859) make alerts more responsive chore: Update Chrome (stable) to 100.0.4896.60 (#20841) Revise test. fix: cy.root respect timeout option. fix(deps): update dependency ansi-regex to v4.1.1 [security] (#20836) chore(deps): update dependency ansi-regex to 4.1.1 [security] (#20807) chore: Refactor cri-client to use async/await (#20825) remove automationId from runnerStore fix firefox automation and adress feedback feat: add automation warning/disconnected states in app

* 10.0-release: (92 commits) chore: remove dependency-tree dep (#20905) chore(launchpad): work on infra for scaffold tests (#20818) fix: build mjs in the cli (#20889) fix(unify): Cypress version link should point to Cypress doc's changelog (#20869) fix: windows build (#20854) fix: add index.mjs to the published files of cli (#20884) refactor: lift indexHtmlFile up to component, add validation (#20870) fix: allow migration of pluginsFile using `env` properties (#20770) fix: viewport from CLI on CT (#20849) Don't communicate if process isn't connected Remove config.get Update packages/data-context/src/data/ProjectLifecycleManager.ts fix: git data source unit test failure (#20875) add comment for autoBindDebug fix: Ensuring current browser is synchronized between app and launchpad (#20830) Fix missed await on merge conflict resolution test(unification): move record keys to contexts (#20860) test: move record keys to contexts (#20859) PR comments Fix tests that visit app without a configured testing type ...

tbiethman added 3 commits March 28, 2022 14:15

fix: Ensuring current browser is synchronized between app and launchpad

985e963

Adding e2e test

f658a87

Fixing types

42f8698

tbiethman requested a review from ZachJW34 March 29, 2022 19:07

tbiethman self-assigned this Mar 29, 2022

Merge branch '10.0-release' into tbiethman/UNIFY-1395-active-browser

9bcd30d

tbiethman commented Mar 29, 2022

View reviewed changes

ZachJW34 previously approved these changes Mar 29, 2022

View reviewed changes

Using slot prop for radio group to determine selection styles

f4ac219

tbiethman dismissed ZachJW34’s stale review via f4ac219 March 29, 2022 19:42

ZachJW34 approved these changes Mar 29, 2022

View reviewed changes

Merge branch '10.0-release' into tbiethman/UNIFY-1395-active-browser

aa53e68

flotwig self-requested a review March 30, 2022 15:33

flotwig approved these changes Mar 30, 2022

View reviewed changes

elevatebart mentioned this pull request Mar 30, 2022

fix: cache last-used browser by project, name, and channel #20760

Merged

1 task

tbiethman added 2 commits March 30, 2022 17:05

Merge branch '10.0-release' into tbiethman/UNIFY-1395-active-browser

69e5fd4

Merge branch '10.0-release' into tbiethman/UNIFY-1395-active-browser

5d0adfe

tbiethman merged commit c4adc8c into 10.0-release Mar 31, 2022

tbiethman deleted the tbiethman/UNIFY-1395-active-browser branch March 31, 2022 15:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: Ensuring current browser is synchronized between app and launchpad #20830

fix: Ensuring current browser is synchronized between app and launchpad #20830

Uh oh!

tbiethman commented Mar 29, 2022 •

edited

Loading

Uh oh!

cypress-bot bot commented Mar 29, 2022

Uh oh!

tbiethman Mar 29, 2022

Uh oh!

tbiethman Mar 29, 2022

Uh oh!

ZachJW34 Mar 29, 2022

Uh oh!

tbiethman Mar 29, 2022

Uh oh!

tbiethman Mar 29, 2022

Uh oh!

tbiethman Mar 29, 2022

Uh oh!

cypress bot commented Mar 29, 2022 •

edited

Loading

Uh oh!

ZachJW34 left a comment

Uh oh!

ZachJW34 Mar 29, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

fix: Ensuring current browser is synchronized between app and launchpad #20830

fix: Ensuring current browser is synchronized between app and launchpad #20830

Uh oh!

Conversation

tbiethman commented Mar 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User facing changelog

Additional details

How has the user experience changed?

PR Tasks

Uh oh!

cypress-bot bot commented Mar 29, 2022

Uh oh!

tbiethman Mar 29, 2022

Choose a reason for hiding this comment

Uh oh!

tbiethman Mar 29, 2022

Choose a reason for hiding this comment

Uh oh!

ZachJW34 Mar 29, 2022

Choose a reason for hiding this comment

Uh oh!

tbiethman Mar 29, 2022

Choose a reason for hiding this comment

Uh oh!

tbiethman Mar 29, 2022

Choose a reason for hiding this comment

Uh oh!

tbiethman Mar 29, 2022

Choose a reason for hiding this comment

Uh oh!

cypress bot commented Mar 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test summary

Run details

Uh oh!

ZachJW34 left a comment

Choose a reason for hiding this comment

Uh oh!

ZachJW34 Mar 29, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tbiethman commented Mar 29, 2022 •

edited

Loading

cypress bot commented Mar 29, 2022 •

edited

Loading