Merge pull request #2148 from broadinstitute/jb-disable-multi-gene-ce…

…llfilter Dynamically hide cell filtering UX when not applicable (SCP-5819)
broadinstitute · Oct 2, 2024 · ef68190 · ef68190
2 parents d3059c8 + 7e9ff2c
commit ef68190
Show file tree

Hide file tree

Showing 4 changed files with 103 additions and 14 deletions.
diff --git a/app/javascript/components/explore/ExploreDisplayPanelManager.jsx b/app/javascript/components/explore/ExploreDisplayPanelManager.jsx
@@ -200,7 +200,7 @@ export default function ExploreDisplayPanelManager({
   showDifferentialExpressionPanel, setIsCellSelecting, currentPointsSelected, isCellSelecting, deGenes,
   setDeGenes, setShowDeGroupPicker,
   cellFaceting, cellFilteringSelection, cellFilterCounts, clusterCanFilter, filterErrorText,
-  updateFilteredCells, panelToShow, toggleViewOptions
+  updateFilteredCells, panelToShow, toggleViewOptions, allowCellFiltering
 }) {
   const [, setRenderForcer] = useState({})
   const [dataCache] = useState(createCache())
@@ -244,14 +244,7 @@ export default function ExploreDisplayPanelManager({
 
   const shownAnnotation = getShownAnnotation(exploreParamsWithDefaults.annotation, annotationList)
 
-  const isSubsampled = exploreParamsWithDefaults.subsample !== 'all'
   let cellFilteringTooltipAttrs = {}
-  if (isSubsampled) {
-    cellFilteringTooltipAttrs = {
-      'data-toggle': 'tooltip',
-      'data-original-title': 'Clicking will remove subsampling; plots might be noticeably slower.'
-    }
-  }
 
   /** Toggle cell filtering panel */
   function toggleCellFilterPanel() {
@@ -359,7 +352,7 @@ export default function ExploreDisplayPanelManager({
                 </button>
               </>
           }
-          {showCellFiltering && panelToShow === 'cell-filtering' &&
+          {showCellFiltering && panelToShow === 'cell-filtering' && allowCellFiltering &&
             <CellFilteringPanelHeader
               togglePanel={togglePanel}
               setShowDifferentialExpressionPanel={setShowDifferentialExpressionPanel}
@@ -370,8 +363,7 @@ export default function ExploreDisplayPanelManager({
               setDeGroupB={setDeGroupB}
               isAuthorDe={isAuthorDe}
               updateFilteredCells={updateFilteredCells}
-              deGenes={deGenes}
-            />
+              deGenes={deGenes}/>
           }
           {panelToShow === 'differential-expression' &&
               <DifferentialExpressionPanelHeader
@@ -447,7 +439,7 @@ export default function ExploreDisplayPanelManager({
                   </div>
                 </>
                 }
-                { showCellFiltering && clusterCanFilter &&
+                { showCellFiltering && clusterCanFilter && allowCellFiltering &&
                   <>
                     <div className="row">
                       <div className={`col-xs-12 cell-filtering-button ${shownTab === 'scatter' && !studyHasDe ? 'create-annotation-cell-filtering' : ''}`}>
@@ -462,7 +454,7 @@ export default function ExploreDisplayPanelManager({
                     </div>
                   </>
                 }
-                { showCellFiltering && !clusterCanFilter &&
+                { showCellFiltering && !clusterCanFilter && allowCellFiltering &&
                   <>
                     <div className="row">
                       <div className={`col-xs-12 cell-filtering-button ${shownTab === 'scatter' && !studyHasDe ? 'create-annotation-cell-filtering' : ''}`}>
@@ -477,6 +469,21 @@ export default function ExploreDisplayPanelManager({
                     </div>
                   </>
                 }
+                { showCellFiltering && !allowCellFiltering &&
+                  <>
+                    <div className="row">
+                      <div className={`col-xs-12 cell-filtering-button ${shownTab === 'scatter' && !studyHasDe ? 'create-annotation-cell-filtering' : ''}`}>
+                        <button
+                          disabled="disabled"
+                          className={`btn btn-primary`}
+                          data-testid="cell-filtering-button-disabled"
+                          data-toggle="tooltip"
+                          data-original-title={`Cell filtering cannot be shown in this context (only scatter or distribution tabs)`}
+                        >Filtering unavailable</button>
+                      </div>
+                    </div>
+                  </>
+                }
                 <SubsampleSelector
                   annotationList={annotationList}
                   cluster={exploreParamsWithDefaults.cluster}

diff --git a/app/javascript/components/explore/ExploreDisplayTabs.jsx b/app/javascript/components/explore/ExploreDisplayTabs.jsx
@@ -302,6 +302,18 @@ export default function ExploreDisplayTabs({
 
   const isCorrelatedScatter = enabledTabs.includes('correlatedScatter')
 
+  // decide whether or not to allow the cell filtering UX
+  // must be in the scatter or distribution tab with less than 2 genes, or using consensus metric
+  const allowCellFiltering = (exploreParams?.genes?.length <= 1 || exploreParams?.consensus !== null) &&
+    ['scatter', 'distribution'].includes(shownTab)
+
+  // hide cell filtering panel in non-applicable settings, but remember state
+  // will reload if user returns to a scenario where cell filtering can be re-applied
+  if (panelToShow === 'cell-filtering' && !allowCellFiltering) {
+    togglePanel('options')
+  } else if (allowCellFiltering && filteredCells && panelToShow === 'options') {
+    togglePanel('cell-filtering')
+  }
 
   // If clustering or annotation changes, then update facets shown for cell filtering
   useEffect(() => {
@@ -696,6 +708,7 @@ export default function ExploreDisplayTabs({
             updateFilteredCells={updateFilteredCells}
             panelToShow={panelToShow}
             toggleViewOptions={toggleViewOptions}
+            allowCellFiltering={allowCellFiltering}
           />
         </div>
       </div>

diff --git a/app/javascript/components/explore/PlotTabs.jsx b/app/javascript/components/explore/PlotTabs.jsx
@@ -55,7 +55,10 @@ export default function PlotTabs({
           const tooltip = disabledTooltips[tabKey]
           const numGenes = tooltip.numToSearch
           const geneText = `gene${tooltip.isMulti ? 's' : ''}`
-          const text = `To show this plot, search ${numGenes} ${geneText} using the box at left`
+          let text = `To show this plot, search ${numGenes} ${geneText} using the box at left`
+          if (['scatter', 'distribution'].includes(tabKey)) {
+            text += " or use the 'Collapse genes by' menu for multiple genes"
+          }
           return (
             <li key={tabKey}
               role="presentation"

diff --git a/test/js/explore/explore-display-tabs.test.js b/test/js/explore/explore-display-tabs.test.js
@@ -26,6 +26,13 @@ jest.mock('lib/cell-faceting', () => {
   }
 })
 
+// Prevent "ReferenceError: $ is not defined" errors when trying to call $(window) functions
+jest.mock('lib/plot', () => {
+  return {
+    getPlotDimensions: jest.fn(() => { return { width: 100, height: 100 } })
+  }
+})
+
 import React from 'react'
 import { render, screen, waitFor } from '@testing-library/react'
 import * as UserProvider from '~/providers/UserProvider'
@@ -424,6 +431,64 @@ describe('explore tabs are activated based on study info and parameters', () =>
     expect(screen.getByTestId('cell-filtering-button')).toHaveTextContent('Filter plotted cells')
   })
 
+  it('hides cell filtering when not applicable', async () => {
+    jest
+      .spyOn(UserProvider, 'getFeatureFlagsWithDefaults')
+      .mockReturnValue({
+        show_cell_facet_filtering: true
+      })
+
+    const newExplore = {
+      'cluster': 'All Cells UMAP',
+      'annotation': {
+        'name': 'biosample_id',
+        'type': 'group',
+        'scope': 'study'
+      },
+      'consensus': null,
+      'genes': ['A1BG', 'A1BG-AS1']
+    }
+    render((
+      <ExploreDisplayTabs
+        studyAccession={'SCP123'}
+        exploreParams={newExplore}
+        exploreParamsWithDefaults={newExplore}
+        exploreInfo={exploreInfoDe}
+      />
+    ))
+    expect(screen.getByTestId('cell-filtering-button-disabled')).toHaveTextContent('Filtering unavailable')
+  })
+
+  it('shows cell filtering when using consensus metric', async () => {
+    jest
+      .spyOn(UserProvider, 'getFeatureFlagsWithDefaults')
+      .mockReturnValue({
+        show_cell_facet_filtering: true
+      })
+
+    const newExplore = {
+      'cluster': 'All Cells UMAP',
+      'annotation': {
+        'name': 'biosample_id',
+        'type': 'group',
+        'scope': 'study'
+      },
+      'consensus': 'median',
+      'genes': ['A1BG', 'A1BG-AS1'],
+      'tab': 'scatter',
+      'facets': ''
+    }
+    render((
+      <ExploreDisplayTabs
+        studyAccession={'SCP123'}
+        exploreParams={newExplore}
+        exploreParamsWithDefaults={newExplore}
+        exploreInfo={exploreInfoDe}
+      />
+    ))
+    expect(screen.getByTestId('cell-filtering-button')).toHaveTextContent('Filter plotted cells')
+  })
+
   it('disables cell filtering button', async () => {
     jest
       .spyOn(UserProvider, 'getFeatureFlagsWithDefaults')
@@ -440,6 +505,7 @@ describe('explore tabs are activated based on study info and parameters', () =>
         clusterCanFilter={false}
         filterErrorText={'Cluster is not indexed'}
         panelToShow={'options'}
+        allowCellFiltering={true}
       />
     )