feat(compass-indexes): adds a signal for notifying when there are too many indexes (#4575)

himanshusinghs · web-flow · commit 1daa01ef1da4 · 2023-07-10T19:05:30.000+02:00
diff --git a/package-lock.json b/package-lock.json
diff --git a/packages/compass-indexes/package.json b/packages/compass-indexes/package.json
@@ -90,7 +90,6 @@
     "react": "^17.0.2",
     "react-dom": "^17.0.2",
     "react-redux": "^8.0.5",
-    "react-virtualized-auto-sizer": "^1.0.6",
     "redux": "^4.2.1",
     "redux-thunk": "^2.4.1",
     "semver": "^5.4.1",
diff --git a/packages/compass-indexes/src/components/indexes-table/indexes-table.spec.tsx b/packages/compass-indexes/src/components/indexes-table/indexes-table.spec.tsx
@@ -106,7 +106,6 @@ const renderIndexList = (
       onDeleteIndex={() => {}}
       onHideIndex={() => {}}
       onUnhideIndex={() => {}}
-      scrollHeight={400}
       {...props}
     />
   );
diff --git a/packages/compass-indexes/src/components/indexes-table/indexes-table.tsx b/packages/compass-indexes/src/components/indexes-table/indexes-table.tsx
@@ -9,6 +9,8 @@ import {
   spacing,
   palette,
   IndexKeysBadge,
+  KeylineCard,
+  useDOMRect,
 } from '@mongodb-js/compass-components';
 
 import TypeField from './type-field';
@@ -69,10 +71,19 @@ const tableStyles = css({
   },
 });
 
+const cardStyles = css({
+  padding: spacing[3],
+});
+
+const spaceProviderStyles = css({
+  flex: 1,
+  position: 'relative',
+  overflow: 'hidden',
+});
+
 type IndexesTableProps = {
   indexes: IndexDefinition[];
   canModifyIndex: boolean;
-  scrollHeight: number;
   serverVersion: string;
   onSortTable: (column: SortColumn, direction: SortDirection) => void;
   onDeleteIndex: (index: IndexDefinition) => void;
@@ -83,14 +94,14 @@ type IndexesTableProps = {
 export const IndexesTable: React.FunctionComponent<IndexesTableProps> = ({
   indexes,
   canModifyIndex,
-  scrollHeight,
   serverVersion,
   onSortTable,
   onDeleteIndex,
   onHideIndex,
   onUnhideIndex,
 }) => {
-  const containerRef = useRef<HTMLDivElement>(null);
+  const cardRef = useRef<HTMLDivElement>(null);
+  const [rectProps, { height: availableHeightInContainer }] = useDOMRect();
   const columns = useMemo(() => {
     const sortColumns: SortColumn[] = [
       'Name and Definition',
@@ -121,80 +132,111 @@ export const IndexesTable: React.FunctionComponent<IndexesTableProps> = ({
 
   useEffect(() => {
     /**
-     * For table header to be sticky, the wrapper element of table needs to have a height.
-     * LG wraps table in a div at multiple levels, so height can not be direclty applied to the wrapper we use.
+     * For table header to be sticky, the wrapper element of table needs to have
+     * a height. LG wraps table in a div at multiple levels, so height can not
+     * be applied directly to the wrapper we have in this markup which is why we
+     * look for the parent element to apply the height.
      */
-    const container =
-      containerRef.current?.getElementsByTagName('table')[0]?.parentElement;
-    if (container) {
-      container.style.height = `${scrollHeight}px`;
+    const table = cardRef.current?.getElementsByTagName('table')[0];
+    const tableParent = table?.parentElement;
+
+    if (table && tableParent) {
+      // We add a top and bottom padding of spacing[3] and our root container
+      // has a bottom margin of spacing[3] which is why the actual usable
+      // height of the container is less than what we get here
+      const heightWithoutSpacing = availableHeightInContainer - spacing[3] * 3;
+
+      // This will be the height of the table. We take whichever is the max of
+      // the actual table height vs the half of the height available to make
+      // sure that our table does not always render in a super small keyline
+      // card when there are only a few rows in the table.
+      const tableHeight = Math.max(
+        table.clientHeight,
+        heightWithoutSpacing / 2
+      );
+
+      // When we have enough space available to render the table, we simply want
+      // our keyline card to have a height as much as that of the table content
+      const tableParentHeight = Math.max(
+        0,
+        Math.min(tableHeight, heightWithoutSpacing)
+      );
+      tableParent.style.height = `${tableParentHeight}px`;
     }
-  }, [scrollHeight]);
+  }, [availableHeightInContainer]);
 
   return (
-    // LG table does not forward ref
-    <div ref={containerRef}>
-      <Table
-        className={tableStyles}
-        data={indexes}
-        columns={columns}
-        data-testid="indexes-list"
-        aria-label="Indexes List Table"
-      >
-        {({ datum: index }) => (
-          <Row
-            key={index.name}
-            data-testid={`index-row-${index.name}`}
-            className={rowStyles}
-          >
-            <Cell data-testid="index-name-field" className={cellStyles}>
-              {index.name}
-            </Cell>
-            <Cell data-testid="index-type-field" className={cellStyles}>
-              <TypeField type={index.type} extra={index.extra} />
-            </Cell>
-            <Cell data-testid="index-size-field" className={cellStyles}>
-              <SizeField size={index.size} relativeSize={index.relativeSize} />
-            </Cell>
-            <Cell data-testid="index-usage-field" className={cellStyles}>
-              <UsageField usage={index.usageCount} since={index.usageSince} />
-            </Cell>
-            <Cell data-testid="index-property-field" className={cellStyles}>
-              <PropertyField
-                cardinality={index.cardinality}
-                extra={index.extra}
-                properties={index.properties}
-              />
-            </Cell>
-            {/* Index actions column is conditional */}
-            {canModifyIndex && (
-              <Cell data-testid="index-actions-field" className={cellStyles}>
-                {index.name !== '_id_' && index.extra.status !== 'inprogress' && (
-                  <div
-                    className={cx(indexActionsCellStyles, 'index-actions-cell')}
-                  >
-                    <IndexActions
-                      index={index}
-                      serverVersion={serverVersion}
-                      onDeleteIndex={onDeleteIndex}
-                      onHideIndex={onHideIndex}
-                      onUnhideIndex={onUnhideIndex}
-                    ></IndexActions>
-                  </div>
-                )}
+    <div className={spaceProviderStyles} {...rectProps}>
+      <KeylineCard ref={cardRef} data-testid="indexes" className={cardStyles}>
+        <Table
+          className={tableStyles}
+          data={indexes}
+          columns={columns}
+          data-testid="indexes-list"
+          aria-label="Indexes List Table"
+        >
+          {({ datum: index }) => (
+            <Row
+              key={index.name}
+              data-testid={`index-row-${index.name}`}
+              className={rowStyles}
+            >
+              <Cell data-testid="index-name-field" className={cellStyles}>
+                {index.name}
+              </Cell>
+              <Cell data-testid="index-type-field" className={cellStyles}>
+                <TypeField type={index.type} extra={index.extra} />
+              </Cell>
+              <Cell data-testid="index-size-field" className={cellStyles}>
+                <SizeField
+                  size={index.size}
+                  relativeSize={index.relativeSize}
+                />
+              </Cell>
+              <Cell data-testid="index-usage-field" className={cellStyles}>
+                <UsageField usage={index.usageCount} since={index.usageSince} />
               </Cell>
-            )}
-            <Row>
-              <Cell
-                className={cx(nestedRowCellStyles, cellStyles)}
-                colSpan={canModifyIndex ? 6 : 5}
-              >
-                <IndexKeysBadge keys={index.fields} />
+              <Cell data-testid="index-property-field" className={cellStyles}>
+                <PropertyField
+                  cardinality={index.cardinality}
+                  extra={index.extra}
+                  properties={index.properties}
+                />
               </Cell>
+              {/* Index actions column is conditional */}
+              {canModifyIndex && (
+                <Cell data-testid="index-actions-field" className={cellStyles}>
+                  {index.name !== '_id_' &&
+                    index.extra.status !== 'inprogress' && (
+                      <div
+                        className={cx(
+                          indexActionsCellStyles,
+                          'index-actions-cell'
+                        )}
+                      >
+                        <IndexActions
+                          index={index}
+                          serverVersion={serverVersion}
+                          onDeleteIndex={onDeleteIndex}
+                          onHideIndex={onHideIndex}
+                          onUnhideIndex={onUnhideIndex}
+                        ></IndexActions>
+                      </div>
+                    )}
+                </Cell>
+              )}
+              <Row>
+                <Cell
+                  className={cx(nestedRowCellStyles, cellStyles)}
+                  colSpan={canModifyIndex ? 6 : 5}
+                >
+                  <IndexKeysBadge keys={index.fields} />
+                </Cell>
+              </Row>
             </Row>
-          </Row>
-        )}
-      </Table>
+          )}
+        </Table>
+      </KeylineCard>
     </div>
   );
 };
diff --git a/packages/compass-indexes/src/components/indexes-toolbar/indexes-toolbar.spec.tsx b/packages/compass-indexes/src/components/indexes-toolbar/indexes-toolbar.spec.tsx
@@ -14,6 +14,7 @@ const renderIndexesToolbar = (
 
   render(
     <IndexesToolbar
+      hasTooManyIndexes={false}
       errorMessage={null}
       isReadonly={false}
       isReadonlyView={false}
@@ -174,4 +175,23 @@ describe('IndexesToolbar Component', function () {
       expect(onRefreshIndexesSpy).to.have.been.calledOnce;
     });
   });
+
+  describe('when there are too many indexes', function () {
+    it('should render insights for too many indexes', function () {
+      renderIndexesToolbar({
+        hasTooManyIndexes: true,
+      });
+      expect(() => screen.getByTestId('insight-badge-button')).to.not.throw;
+    });
+
+    context('and when there is an error', function () {
+      it('should not render insights', function () {
+        renderIndexesToolbar({
+          hasTooManyIndexes: true,
+          errorMessage: 'Something bad happened',
+        });
+        expect(() => screen.getByTestId('insight-badge-button')).to.throw;
+      });
+    });
+  });
 });
diff --git a/packages/compass-indexes/src/components/indexes-toolbar/indexes-toolbar.tsx b/packages/compass-indexes/src/components/indexes-toolbar/indexes-toolbar.tsx
@@ -9,8 +9,10 @@ import {
   spacing,
   Icon,
   SpinLoader,
+  SignalPopover,
 } from '@mongodb-js/compass-components';
 import type AppRegistry from 'hadron-app-registry';
+import { usePreference } from 'compass-preferences-model';
 
 const containerStyles = css({
   margin: `${spacing[3]}px 0`,
@@ -20,7 +22,8 @@ const toolbarButtonsContainer = css({
   display: 'flex',
   flexDirection: 'row',
   gap: spacing[2],
-  justifyContent: 'flex-end',
+  justifyContent: 'flex-start',
+  alignItems: 'center',
 });
 
 const errorStyles = css({ marginTop: spacing[2] });
@@ -36,6 +39,7 @@ type IndexesToolbarProps = {
   isReadonly: boolean;
   isReadonlyView: boolean;
   isWritable: boolean;
+  hasTooManyIndexes: boolean;
   localAppRegistry: AppRegistry;
   isRefreshing: boolean;
   writeStateDescription?: string;
@@ -51,9 +55,11 @@ export const IndexesToolbar: React.FunctionComponent<IndexesToolbarProps> = ({
   localAppRegistry,
   isRefreshing,
   writeStateDescription,
+  hasTooManyIndexes,
   onRefreshIndexes,
   readOnly, // preferences readOnly.
 }) => {
+  const showInsights = usePreference('showInsights', React) && !errorMessage;
   const onClickCreateIndex = useCallback(() => {
     localAppRegistry.emit('open-create-index-modal');
   }, [localAppRegistry]);
@@ -72,16 +78,6 @@ export const IndexesToolbar: React.FunctionComponent<IndexesToolbarProps> = ({
       {!isReadonlyView && (
         <div data-testid="indexes-toolbar">
           <div className={toolbarButtonsContainer}>
-            <Button
-              data-testid="refresh-indexes-button"
-              disabled={isRefreshing}
-              onClick={() => onRefreshIndexes()}
-              variant="default"
-              size="small"
-              leftGlyph={refreshButtonIcon}
-            >
-              Refresh
-            </Button>
             {showCreateIndexButton && (
               <Tooltip
                 enabled={!isWritable}
@@ -112,6 +108,28 @@ export const IndexesToolbar: React.FunctionComponent<IndexesToolbarProps> = ({
                 {writeStateDescription}
               </Tooltip>
             )}
+            <Button
+              data-testid="refresh-indexes-button"
+              disabled={isRefreshing}
+              onClick={() => onRefreshIndexes()}
+              variant="default"
+              size="small"
+              leftGlyph={refreshButtonIcon}
+            >
+              Refresh
+            </Button>
+            {showInsights && hasTooManyIndexes && (
+              <SignalPopover
+                signals={{
+                  id: 'too-many-indexes',
+                  title: 'High number of indexes on collection',
+                  description:
+                    'Consider reviewing your indexes to remove any that are unnecessary. Learn more about this anti-pattern',
+                  learnMoreLink:
+                    'https://www.mongodb.com/docs/manual/core/data-model-operations/#indexes',
+                }}
+              />
+            )}
           </div>
         </div>
       )}
diff --git a/packages/compass-indexes/src/components/indexes/indexes.tsx b/packages/compass-indexes/src/components/indexes/indexes.tsx
