SWC-6910 - Entity Finder 'Selected' tab #1442

nickgros · 2024-12-05T18:44:02Z

Update EntityFinder to show selected items in a tab instead of a lower pane
Updated EntityFinder selection to use a Map of entity ID -> Reference instead of entity ID -> version number. With this change, we can remove hacky behavior to store a fake 'null' version number in all places except one case where it is used as an value
Update styles to more closely match design
Update EntityFinder tests to be type-checked

See the EntityFinderModal story for a demo

nickgros · 2024-12-05T18:45:04Z

packages/synapse-react-client/src/components/EntityFileBrowser/EntityFileBrowser.tsx

@@ -66,7 +66,7 @@ export const EntityFileBrowser: React.FunctionComponent<
    },
  }
  return (
-    <div className="EntityFinderReflexContainer">
+    <div className="EntityFileBrowser EntityFinderReflexContainer">


Added class name to apply cascaded styles

nickgros · 2024-12-05T18:45:26Z

packages/synapse-react-client/src/components/EntityFinder/EntityFinder.stories.ts

-    selectedCopy: count => {
-      return `${count} Item${count > 1 ? 's' : ''} Selected`
-    },


Removed now-unused selectedCopy prop.

nickgros · 2024-12-05T18:45:40Z

packages/synapse-react-client/src/components/EntityFinder/EntityFinder.test.tsx

@@ -10,7 +10,6 @@ import {
 } from '../../../src/components/EntityFinder/details/EntityDetailsList'


Moved this file so it is now type-checked

nickgros · 2024-12-05T18:46:20Z

packages/synapse-react-client/src/components/EntityFinder/EntityFinder.test.tsx

@@ -355,61 +361,24 @@ describe('EntityFinder tests', () => {
  })

  describe('Search', () => {
-    it('Updates the search button text when only one type is selectable', async () => {


The search input field is no longer hidden behind a button press, so many of these search tests are no longer relevant.

nickgros · 2024-12-05T18:48:53Z

packages/synapse-react-client/src/components/EntityFinder/EntityFinder.tsx

-// In the map used to track selections, we use -1 to denote 'selected without version'
-// This is necessary because undefined is returned by map.get when the item is not in the map
-export const NO_VERSION_NUMBER = -1


Updated the selected type from a map of (entityID, versionNumber) to a map of (entityID, Reference), so we can remove this hacky convention

nickgros · 2024-12-05T18:49:12Z

packages/synapse-react-client/src/components/EntityFinder/EntityFinder.tsx

-  /** The text to show before the list of selected entities */
-  selectedCopy?: string | ((count: number) => string)


prop is no longer used after these changes

nickgros · 2024-12-05T18:52:55Z

packages/synapse-react-client/src/components/EntityFinder/EntityFinder.tsx

-            {searchActive ? (
-              <Button
-                variant="outlined"
-                color="primary"
-                onClick={() => {
-                  setSearchActive(false)
-                  setSearchInput('')
-                  setSearchTerms(undefined)
-                }}
-                startIcon={<ArrowBackOutlinedIcon />}
-                sx={{ flexShrink: 0, mr: 1 }}
-              >
-                Back to Browse
-              </Button>
-            ) : (
-              <Button
-                variant="contained"
-                color="primary"
-                className="EntityFinder__Search__SearchButton"
-                onClick={() => {
-                  setSearchActive(true)
-                  searchInputRef.current!.focus()
-                }}
-                startIcon={<SearchIcon />}
-                sx={{ flexShrink: 0 }}
-              >
-                {searchButtonText}
-              </Button>
-            )}


Removed these buttons to show/hide search. The new design has the search input always visible.

nickgros · 2024-12-05T18:54:00Z

packages/synapse-react-client/src/components/EntityFinder/EntityFinder.tsx

+              endAdornment: searchTerms ? (
+                <InputAdornment position="end">
+                  <IconButton
+                    size={'small'}
+                    onClick={() => {
+                      setSearchInput('')
+                      setSearchTerms(undefined)
+                      setSearchActive(false)
+                    }}
+                    aria-label="Clear Search"
+                    disabled={!searchInput && !searchActive}
+                  >
+                    <ClearIcon />
+                  </IconButton>
+                </InputAdornment>
+              ) : undefined,


Still show a clear button to return to the file browser from the search state

nickgros · 2024-12-05T18:54:35Z

packages/synapse-react-client/src/components/EntityFinder/EntityFinder.tsx

+        <div
+          role={'tabpanel'}
+          style={{
+            display: currentTab === EntityFinderTab.SELECTED ? 'block' : 'none',
+          }}
+        >
+          <EntityDetailsList
+            {...entityDetailsListProps}
+            configuration={{
+              type: EntityDetailsListDataConfigurationType.REFERENCE_LIST,
+              referenceList: Array.from(selectedEntitiesSnapshot.values()),
+            }}
          />
-        )}
+        </div>


Show the selected items in an EntityDetailsList

nickgros · 2024-12-05T18:56:10Z

...synapse-react-client/src/components/EntityFinder/details/configurations/ReferenceDetails.tsx

+  references: Reference[]
+}
+
+export const ReferenceDetails: React.FunctionComponent<SearchDetailsProps> = ({


The selected items will be provided as References. This component translates the reference list to entity headers, which can be passed into DetailsView to render the table.

nickgros · 2024-12-05T18:56:23Z

packages/synapse-react-client/src/components/EntityFinder/details/view/DetailsView.test.tsx

@@ -1,40 +1,36 @@
 import '@testing-library/jest-dom'


File is now type-checked

nickgros · 2024-12-05T18:57:22Z

.../synapse-react-client/src/components/EntityFinder/details/view/DetailsViewTableRenderers.tsx

+            <option value={NO_VERSION_NUMBER_OPTION_VALUE}>
+              {latestVersionText}
+            </option>


The only place we have to maintain the "NO_VERSION_NUMBER" = -1 convention, because an <option value cannot be null

nickgros · 2024-12-05T18:58:18Z

packages/synapse-react-client/src/components/EntityFinder/tree/EntityTree.tsx

+  useEffect(() => {
+    if (
+      FinderScope.CURRENT_PROJECT === scope &&
+      projectId &&
+      !isLoadingProjectHeader &&
+      !projectHeader
+    ) {
+      // The header wasn't returned, so the user doesn't have access to the current project
+      // Let's change the scope to something else
+      displayToast(
+        `You don't have access to the current project (${projectId}).`,
+        'warning',
+      )
+      setScope(FinderScope.CREATED_BY_ME)
+    }
+  }, [isLoadingProjectHeader, projectHeader, projectId, scope])


Put this in an effect to prevent firing every render

nickgros · 2024-12-05T18:58:38Z

packages/synapse-react-client/src/components/EntityFinder/tree/EntityTree.tsx

-          <div className="Browse">Browse:</div>
-          <div onClick={e => e.stopPropagation()}>


Removed "Browse" copy

nickgros · 2024-12-05T19:10:35Z

packages/synapse-react-client/src/components/EntityFinder/tree/VirtualizedTree.tsx

+      <Tooltip title={tooltipContent} placement="right">
+        <div className="EntityName" ref={ref}>
          {nodeText}
-        </Tooltip>
-      </div>
+        </div>
+      </Tooltip>


Wrap tooltip around the div so that it is positioned to the right of the visible text, not to the right of the overflowing & not visible text.

nickgros · 2024-12-05T19:11:08Z

packages/synapse-react-client/src/components/EntityFinder/tree/VirtualizedTree.tsx

+        <>
+          {isLoading ? (
+            <SynapseSpinner size={10} />
+          ) : (
+            <button
+              aria-label={isOpen ? 'Collapse' : 'Expand'}
+              onClick={event => {
+                event.stopPropagation()
+                toggleExpand()
+              }}
+            >
+              <ExpandIcon className="ExpandButton" />
+            </button>
+          )}
+        </>


Update icon

packages/synapse-react-client/src/components/EntityFinder/useEntitySelection.ts

- Update EntityFinder to show selected items in a tab instead of a lower pane - Updated EntityFinder selection to use a Map of entity ID -> Reference instead of entity ID -> version number. With this change, we can remove hacky behavior to store a fake 'null' version number in all places except one case where it is used as an <option> value - Update styles to more closely match design - Update EntityFinder tests to be type-checked

nickgros · 2024-12-05T19:28:40Z

packages/synapse-react-client/src/style/components/entity_finder/_entity-finder.scss

+.MuiDialog-root {
+  .EntityFinder {
+    &__MainPanel.MainPanelDualPane,
+    &__MainPanel.MainPanelSearch {
+      // Hack to make the dialog responsive to the width of the entity finder
+      // We may be able to remove this if we remove/replace the EntityFinder's `AutoSizer` component with something other
+      // than the 'react-base-table' implementation
+      width: 85vw;
+    }
+  }
+}


Extract this style to only apply it in a dialog. Otherwise, we can make the width of the entity finder 100%

nickgros · 2024-12-05T19:29:54Z

packages/synapse-react-client/src/style/components/entity_finder/_entity-finder.scss

+  --entity-finder-height: #{$-finder-height};
+  --entity-tree-header-height: 65px;


Fix a calculation issue in the experimental EntityFileBrowser (where --entity-tree-header-height should be 0px) by creating CSS variables with these values.

Without this change, the EntityFileBrowser appears to have empty space in the tree that overflowing content should be using

nickgros · 2024-12-05T19:30:15Z

packages/synapse-react-client/src/style/components/entity_finder/_entity-finder.scss

-  &__Search__Input {
-    transition: flex-grow 250ms ease-in-out, width 250ms ease-in-out;
-    &[data-active='true'] {
-      flex-grow: 1;
-      width: 100%;
-    }
-    &[data-active='false'] {
-      flex-grow: 0.00001;
-      overflow: hidden;
-      width: 0;
-    }
-  }


Interaction that used these styles was removed

nickgros · 2024-12-05T19:30:31Z

packages/synapse-react-client/src/style/components/entity_finder/_entity-finder.scss

+  .TreeView.BrowseTree {
+    padding-left: 20px;
+  }


Do not apply this padding to the EntityFileBrowser

nickgros · 2024-12-05T19:30:56Z

packages/synapse-react-client/src/style/components/entity_finder/_tree-node-browse.scss

  .Node.BrowseNode {
-    grid-template-columns: [toggle] 20px [name] auto;
+    grid-template-columns: [toggle] 20px [name] minmax(0, max-content);


Fixes entity name overflow

nickgros · 2024-12-05T19:31:21Z

packages/synapse-react-client/src/style/components/entity_finder/_tree-node-select.scss

+      grid-template-columns: [toggle] 15px [icon] 27px [name] minmax(
+          0,
+          max-content
+        ) [badge] auto;


Fixes entity name overflow

…tories

nickgros · 2024-12-06T16:53:27Z

packages/synapse-react-client/src/components/EntityFinder/EntityFinder.tsx

+    selectColumnType: selectMultiple ? 'checkbox' : 'none',
+    toggleSelection: toggleSelection,
+    enableSelectAll: selectMultiple,
+    setCurrentContainer: setCurrentContainer,
  }

  return (
    <SynapseErrorBoundary>
      <div className="EntityFinder">
        <Box


Updated this flex container to include the tabs & the search input. This section is only styled using MUI, SCSS does not touch these

nickgros · 2024-12-06T16:58:43Z

packages/synapse-react-client/src/components/EntityFinder/EntityFinder.tsx

+          sx={{
+            flexWrap: { xs: 'wrap', md: 'nowrap' },
+          }}


Just kinda tweaked this responsive layout with the search bar until it felt good

2024-12-06.11-57-42.mp4

I think this is fine, but there may be some confusion on the scope of this search box (users may think this is searching the selected entities table, for example).
The real mobile responsive issue is the browse UX, which I think is out of scope for this ticket

Great point

jay-hodgson · 2024-12-06T17:53:49Z

packages/synapse-react-client/src/components/EntityFinder/EntityFinder.tsx

+            onKeyDown={(event: any) => {
+              if (event.key === 'Enter') {
+                const trimmedInput = event.target.value.trim()
+                setSearchActive(!!trimmedInput)


@nickgros : On search, I think we should change to the Browse tab. When I'm on the Selected tab and do a search, the search results are not shown (I need to know to switch tabs to see the results)

Good catch, and that's a good idea--in general this is weird so I will probably follow-up with design about this.

from code review

nickgros commented Dec 5, 2024

View reviewed changes

nickgros force-pushed the SWC-6910 branch from baf8856 to 7604f58 Compare December 5, 2024 19:09

nickgros commented Dec 5, 2024

View reviewed changes

packages/synapse-react-client/src/components/EntityFinder/useEntitySelection.ts Outdated Show resolved Hide resolved

nickgros force-pushed the SWC-6910 branch from a75bf63 to 246b59a Compare December 5, 2024 19:27

nickgros commented Dec 5, 2024

View reviewed changes

nickgros added the chromatic Deploys the latest commit on this PR to Chromatic and runs snapshot tests label Dec 5, 2024

SWC-6910 - Expand dialog when switching to the selected tab

d7627d3

nickgros added chromatic Deploys the latest commit on this PR to Chromatic and runs snapshot tests and removed chromatic Deploys the latest commit on this PR to Chromatic and runs snapshot tests labels Dec 6, 2024

SWC-6910 - Tweak Entity Finder search box responsive styles, update s…

b5cbff7

…tories

nickgros added chromatic Deploys the latest commit on this PR to Chromatic and runs snapshot tests and removed chromatic Deploys the latest commit on this PR to Chromatic and runs snapshot tests labels Dec 6, 2024

nickgros commented Dec 6, 2024

View reviewed changes

jay-hodgson reviewed Dec 6, 2024

View reviewed changes

SWC-6910 - On search, switch to the Browse tab

e057f2e

from code review

nickgros added chromatic Deploys the latest commit on this PR to Chromatic and runs snapshot tests and removed chromatic Deploys the latest commit on this PR to Chromatic and runs snapshot tests labels Dec 9, 2024

nickgros mentioned this pull request Dec 9, 2024

SWC-6910 - EntityFinder prop was removed Sage-Bionetworks/SynapseWebClient#5602

Merged

jay-hodgson approved these changes Dec 9, 2024

View reviewed changes

Merge branch 'main' into SWC-6910

5e87476

nickgros merged commit 9cb7ad9 into Sage-Bionetworks:main Dec 9, 2024
7 of 19 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SWC-6910 - Entity Finder 'Selected' tab #1442

SWC-6910 - Entity Finder 'Selected' tab #1442

nickgros commented Dec 5, 2024 •

edited

Loading

nickgros Dec 5, 2024

nickgros Dec 5, 2024

nickgros Dec 5, 2024

nickgros Dec 5, 2024

nickgros Dec 5, 2024

nickgros Dec 5, 2024

nickgros Dec 5, 2024 •

edited

Loading

nickgros Dec 5, 2024

nickgros Dec 5, 2024

nickgros Dec 5, 2024

nickgros Dec 5, 2024

nickgros Dec 5, 2024

nickgros Dec 5, 2024

nickgros Dec 5, 2024

nickgros Dec 5, 2024 •

edited

Loading

nickgros Dec 5, 2024

nickgros Dec 5, 2024

nickgros Dec 5, 2024 •

edited

Loading

nickgros Dec 5, 2024

nickgros Dec 5, 2024

nickgros Dec 5, 2024

nickgros Dec 5, 2024

nickgros Dec 6, 2024

nickgros Dec 6, 2024 •

edited

Loading

jay-hodgson Dec 6, 2024

nickgros Dec 6, 2024

jay-hodgson Dec 6, 2024

nickgros Dec 6, 2024 •

edited

Loading

		@@ -10,7 +10,6 @@ import {
		} from '../../../src/components/EntityFinder/details/EntityDetailsList'

		/** The text to show before the list of selected entities */
		selectedCopy?: string \| ((count: number) => string)

		<div className="Browse">Browse:</div>
		<div onClick={e => e.stopPropagation()}>

		--entity-finder-height: #{$-finder-height};
		--entity-tree-header-height: 65px;

SWC-6910 - Entity Finder 'Selected' tab #1442

SWC-6910 - Entity Finder 'Selected' tab #1442

Conversation

nickgros commented Dec 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nickgros Dec 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nickgros Dec 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nickgros Dec 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nickgros Dec 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nickgros Dec 6, 2024 • edited Loading

Choose a reason for hiding this comment

nickgros commented Dec 5, 2024 •

edited

Loading

nickgros Dec 5, 2024 •

edited

Loading

nickgros Dec 5, 2024 •

edited

Loading

nickgros Dec 5, 2024 •

edited

Loading

nickgros Dec 6, 2024 •

edited

Loading

nickgros Dec 6, 2024 •

edited

Loading