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

tbiethman · web-flow · commit c4adc8cf09e4 · 2022-03-31T10:25:15.000-05:00
* fix: Ensuring current browser is synchronized between app and launchpad

* Adding e2e test

* Fixing types

* Using slot prop for radio group to determine selection styles
diff --git a/packages/app/src/runner/SpecRunnerHeaderOpenMode.vue b/packages/app/src/runner/SpecRunnerHeaderOpenMode.vue
@@ -171,26 +171,6 @@ fragment SpecRunnerHeader_Browser on Browser {
 }
 `
 
-gql`
-mutation SpecRunnerHeader_SetBrowser($browserId: ID!, $specPath: String!) {
-  launchpadSetBrowser(id: $browserId) {
-    id
-    currentBrowser {
-      id
-      displayName
-      majorVersion
-    }
-    browsers {
-      id
-      isSelected
-    }
-  }
-  launchOpenProject(specPath: $specPath) {
-    id
-  }
-}
-`
-
 const { t } = useI18n()
 
 const autStore = useAutStore()
diff --git a/packages/data-context/src/actions/DataEmitterActions.ts b/packages/data-context/src/actions/DataEmitterActions.ts
@@ -28,6 +28,10 @@ abstract class DataEmitterEvents {
     this._emit('cloudViewerChange')
   }
 
+  /**
+   * Emitted when the browserStatus field has changed due to the browser
+   * having opened or closed.
+   */
   browserStatusChange () {
     this._emit('browserStatusChange')
   }
diff --git a/packages/launchpad/cypress/e2e/choose-a-browser.cy.ts b/packages/launchpad/cypress/e2e/choose-a-browser.cy.ts
@@ -1,3 +1,5 @@
+import type { FoundBrowser } from '@packages/types/src'
+
 describe('Choose a Browser Page', () => {
   beforeEach(() => {
     cy.scaffoldProject('launchpad')
@@ -174,8 +176,8 @@ describe('Choose a Browser Page', () => {
 
       cy.get('h1').should('contain', 'Choose a Browser')
 
-      cy.withCtx((ctx) => {
-        ctx.actions.project.launchProject = sinon.spy()
+      cy.withCtx((ctx, o) => {
+        ctx.actions.project.launchProject = o.sinon.spy()
       })
 
       cy.intercept('mutation-OpenBrowser_LaunchProject', cy.stub().as('launchProject'))
@@ -217,8 +219,8 @@ describe('Choose a Browser Page', () => {
 
       cy.intercept('mutation-OpenBrowser_FocusActiveBrowserWindow').as('focusBrowser')
 
-      cy.withCtx((ctx) => {
-        sinon.spy(ctx.actions.browser, 'focusActiveBrowserWindow')
+      cy.withCtx((ctx, o) => {
+        o.sinon.spy(ctx.actions.browser, 'focusActiveBrowserWindow')
       })
 
       cy.get('@focusButton').click()
@@ -238,9 +240,9 @@ describe('Choose a Browser Page', () => {
       })
 
       cy.openProject('launchpad', ['--e2e'])
-      cy.withCtx((ctx) => {
+      cy.withCtx((ctx, o) => {
         ctx.project.setRelaunchBrowser(true)
-        ctx.actions.project.launchProject = sinon.stub()
+        ctx.actions.project.launchProject = o.sinon.stub()
       })
 
       cy.visitLaunchpad()
@@ -251,6 +253,34 @@ describe('Choose a Browser Page', () => {
         expect(ctx.actions.project.launchProject).to.have.been.called
       })
     })
+
+    it('subscribes to changes to browserStatus/currentBrowser through the browserStatusUpdated subscription', () => {
+      cy.openProject('launchpad', ['--e2e'])
+
+      cy.visitLaunchpad()
+
+      cy.get('h1').should('contain', 'Choose a Browser')
+
+      cy.findByRole('radio', { name: 'Chrome v1', checked: true }).as('chromeItem')
+
+      cy.contains('button', 'Start E2E Testing in Chrome').should('be.visible').click()
+
+      cy.withCtx((ctx) => {
+        ctx.browser.setBrowserStatus('open')
+      })
+
+      cy.contains('button', 'Running Chrome')
+
+      // Updating active browser in conjunction with the browser status to ensure that changes to
+      // both are reflected in the UI.
+      cy.withCtx((ctx) => {
+        ctx.actions.app.setActiveBrowser(ctx.lifecycleManager.browsers!.find((browser) => browser.name === 'firefox') as FoundBrowser)
+        ctx.browser.setBrowserStatus('closed')
+      })
+
+      cy.contains('button', 'Start E2E Testing in Firefox').should('be.visible')
+      cy.findByRole('radio', { name: 'Firefox v5', checked: true }).should('be.visible')
+    })
   })
 
   describe('No System Browsers Detected', () => {
diff --git a/packages/launchpad/src/setup/OpenBrowserList.cy.tsx b/packages/launchpad/src/setup/OpenBrowserList.cy.tsx
@@ -14,9 +14,6 @@ describe('<OpenBrowserList />', () => {
 
   it('renders a long list of found browsers correctly', () => {
     cy.mountFragment(OpenBrowserListFragmentDoc, {
-      onResult: (result) => {
-        result.currentBrowser = null
-      },
       render: (gqlVal) =>
         (<div class="border-current border-1 resize overflow-auto">
           <OpenBrowserList gql={gqlVal}/>
@@ -36,19 +33,11 @@ describe('<OpenBrowserList />', () => {
     cy.get('[data-cy-browser="fake"]').should('have.attr', 'aria-disabled', 'true')
     cy.get('[data-cy-browser="fake"] img').should('have.attr', 'src').should('include', 'generic-browser')
 
-    // If no default value, should choose electron
-    cy.get('[data-cy-browser="electron"]').should('have.attr', 'aria-checked', 'true')
-    cy.get('[data-cy="launch-button"]').contains(defaultMessages.openBrowser.startE2E.replace('{browser}', 'Electron'))
-    cy.get('[data-cy="launch-button"]').get('[data-cy="icon-testing-type-e2e"]')
-
     cy.percySnapshot()
   })
 
   it('displays a tooltip for an unsupported browser', () => {
     cy.mountFragment(OpenBrowserListFragmentDoc, {
-      onResult: (result) => {
-        result.currentBrowser = null
-      },
       render: (gqlVal) =>
         (<div class="border-current border-1 resize overflow-auto">
           <div class="h-40" />
@@ -122,21 +111,42 @@ describe('<OpenBrowserList />', () => {
     cy.mountFragment(OpenBrowserListFragmentDoc, {
       onResult: (res) => {
         res.browserStatus = 'open'
-        res.currentBrowser = longBrowsersList.find((browser) => !browser.isFocusSupported) || null
+        res.currentBrowser!.isFocusSupported = false
+      },
+      render: (gqlVal) => {
+        return (
+          <div class="border-current border-1 resize overflow-auto">
+            <OpenBrowserList
+              gql={gqlVal}
+              onCloseBrowser={cy.stub().as('closeBrowser')}/>
+          </div>)
       },
-      render: (gqlVal) => (
-        <div class="border-current border-1 resize overflow-auto">
-          <OpenBrowserList
-            gql={gqlVal}
-            onCloseBrowser={cy.stub().as('closeBrowser')}/>
-        </div>),
     })
 
-    cy.get('[data-cy-browser]').each((browser) => cy.wrap(browser).should('have.attr', 'aria-disabled', 'true'))
     cy.contains('button', defaultMessages.openBrowser.running.replace('{browser}', 'Electron')).should('be.disabled')
     cy.contains('button', defaultMessages.openBrowser.focus).should('not.exist')
-    cy.contains('button', defaultMessages.openBrowser.close).click()
-    cy.get('@closeBrowser').should('have.been.called')
+
+    cy.percySnapshot()
+  })
+
+  it('hides action buttons when currentBrowser is null', () => {
+    cy.mountFragment(OpenBrowserListFragmentDoc, {
+      onResult: (res) => {
+        res.currentBrowser = null
+      },
+      render: (gqlVal) => {
+        return (
+          <div class="border-current border-1 resize overflow-auto">
+            <OpenBrowserList
+              gql={gqlVal}
+              onCloseBrowser={cy.stub().as('closeBrowser')}/>
+          </div>)
+      },
+    })
+
+    cy.get('button[data-cy="launch-button"]').should('not.exist')
+    cy.contains('button', defaultMessages.openBrowser.focus).should('not.exist')
+    cy.contains('button', defaultMessages.openBrowser.close).should('not.exist')
 
     cy.percySnapshot()
   })
diff --git a/packages/launchpad/src/setup/OpenBrowserList.vue b/packages/launchpad/src/setup/OpenBrowserList.vue
@@ -55,7 +55,7 @@
           </div>
           <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': checked, 'text-gray-500': browser.disabled || !browser.isVersionSupported }"
           >
             {{ browser.displayName }}
           </div>
@@ -70,68 +70,69 @@
       class="mb-14"
     >
       <div class="flex mb-4 gap-16px items-center justify-center">
-        <template v-if="browserStatus.closed || browserStatus.opening">
-          <Button
-            v-if="browserStatus.closed"
-            size="lg"
-            type="submit"
-            :prefix-icon="testingTypeIcon"
-            prefix-icon-class="icon-dark-white"
-            variant="secondary"
-            data-cy="launch-button"
-            class="font-medium"
-          >
-            {{ browserText[props.gql.currentTestingType].start }}
-          </Button>
-          <Button
-            v-else
-            size="lg"
-            type="button"
-            disabled
-            variant="pending"
-            class="font-medium disabled:cursor-default"
-            :prefix-icon="StatusRunningIcon"
-            prefix-icon-class="icon-light-gray-300 icon-dark-white animate-spin"
-          >
-            {{ browserText[props.gql.currentTestingType].opening }}
-          </Button>
-        </template>
-
-        <template v-else>
-          <Button
-            size="lg"
-            type="button"
-            disabled
-            variant="pending"
-            :prefix-icon="testingTypeIcon"
-            prefix-icon-class="icon-dark-white"
-            class="font-medium disabled:cursor-default"
-          >
-            {{ browserText.running }}
-          </Button>
-          <Button
-            v-if="props.gql.currentBrowser?.isFocusSupported"
-            size="lg"
-            type="button"
-            variant="outline"
-            :prefix-icon="ExportIcon"
-            prefix-icon-class="icon-dark-gray-500"
-            class="font-medium"
-            @click="emit('focusBrowser')"
-          >
-            {{ browserText.focus }}
-          </Button>
-          <Button
-            size="lg"
-            type="button"
-            variant="outline"
-            :prefix-icon="PowerStandbyIcon"
-            prefix-icon-class="icon-dark-gray-500"
-            class="font-medium"
-            @click="emit('closeBrowser')"
-          >
-            {{ browserText.close }}
-          </Button>
+        <template v-if="selectedBrowserId">
+          <template v-if="browserStatus.closed || browserStatus.opening">
+            <Button
+              v-if="browserStatus.closed"
+              size="lg"
+              type="submit"
+              :prefix-icon="testingTypeIcon"
+              prefix-icon-class="icon-dark-white"
+              variant="secondary"
+              data-cy="launch-button"
+              class="font-medium"
+            >
+              {{ browserText[props.gql.currentTestingType].start }}
+            </Button>
+            <Button
+              v-else
+              size="lg"
+              type="button"
+              disabled
+              variant="pending"
+              class="font-medium disabled:cursor-default"
+              :prefix-icon="StatusRunningIcon"
+              prefix-icon-class="icon-light-gray-300 icon-dark-white animate-spin"
+            >
+              {{ browserText[props.gql.currentTestingType].opening }}
+            </Button>
+          </template>
+          <template v-else>
+            <Button
+              size="lg"
+              type="button"
+              disabled
+              variant="pending"
+              :prefix-icon="testingTypeIcon"
+              prefix-icon-class="icon-dark-white"
+              class="font-medium disabled:cursor-default"
+            >
+              {{ browserText.running }}
+            </Button>
+            <Button
+              v-if="props.gql.currentBrowser?.isFocusSupported"
+              size="lg"
+              type="button"
+              variant="outline"
+              :prefix-icon="ExportIcon"
+              prefix-icon-class="icon-dark-gray-500"
+              class="font-medium"
+              @click="emit('focusBrowser')"
+            >
+              {{ browserText.focus }}
+            </Button>
+            <Button
+              size="lg"
+              type="button"
+              variant="outline"
+              :prefix-icon="PowerStandbyIcon"
+              prefix-icon-class="icon-dark-gray-500"
+              class="font-medium"
+              @click="emit('closeBrowser')"
+            >
+              {{ browserText.close }}
+            </Button>
+          </template>
         </template>
       </div>
 
@@ -182,21 +183,14 @@ fragment OpenBrowserList on CurrentProject {
   id
   currentBrowser {
     id
-    displayName
-    path
     isFocusSupported
   }
   browsers {
     id
-    name
-    family
     disabled
-    isSelected
     isVersionSupported
-    channel
+    name
     displayName
-    path
-    version
     warning
     majorVersion
   }
@@ -208,6 +202,10 @@ subscription OpenBrowserList_browserStatusChange {
   browserStatusChange {
     id
     browserStatus
+    currentBrowser {
+      id
+      isFocusSupported
+    }
   }
 }
 `
@@ -238,7 +236,11 @@ const browsers = computed(() => {
 const setBrowser = useMutation(OpenBrowserList_SetBrowserDocument)
 
 const selectedBrowserId = computed({
-  get: () => props.gql.currentBrowser?.id || props.gql.browsers?.find((browser) => browser.displayName === 'Electron')?.id,
+  get: () => {
+    // NOTE: The currentBrowser is set to the first detected browser
+    // found during project initialization. It should always be defined.
+    return props.gql.currentBrowser?.id
+  },
   set (browserId) {
     if (browserId) {
       setBrowser.executeMutation({ id: browserId })

Original file line number	Diff line number	Diff line change
`@@ -28,6 +28,10 @@ abstract class DataEmitterEvents {`
`28`	`28`	`this._emit('cloudViewerChange')`
`29`	`29`	`}`
`30`	`30`
	`31`	`+ /**`
	`32`	`+ * Emitted when the browserStatus field has changed due to the browser`
	`33`	`+ * having opened or closed.`
	`34`	`+ */`
`31`	`35`	`browserStatusChange () {`
`32`	`36`	`this._emit('browserStatusChange')`
`33`	`37`	`}`