feat(insights): enable insights by default COMPASS-7000 (#4622)

* feat(insights): enable insights by default

* chore(components): clarify comment

Author: gribnoysup

packages/compass-components/src/components/signal-popover.tsx

@@ -16,6 +16,7 @@ import { useDarkMode } from '../hooks/use-theme';
 import { spacing } from '@leafygreen-ui/tokens';
 import { GuideCue } from './guide-cue/guide-cue';
 import { useEffectOnChange } from '../hooks/use-effect-on-change';
+import { rafraf } from '../utils/rafraf';
 
 type SignalTrackingHooks = {
   onSignalMount(id: string): void;
@@ -75,6 +76,8 @@ const SignalHooksProvider: React.FunctionComponent<
   );
 };
 
+const TRANSITION_DURATION_MS = 150;
+
 export type Signal = {
   /**
    * Unique signal id that will be used to resolve the dismissing logic.
@@ -336,7 +339,7 @@ const transitionStyles = css({
   transitionProperty:
     'opacity, width, border-radius, color, box-shadow, background-color',
   transitionTimingFunction: 'linear',
-  transitionDuration: '0.15s',
+  transitionDuration: `${TRANSITION_DURATION_MS}ms`,
 });
 
 const badgeStyles = css(
@@ -438,16 +441,16 @@ const SignalPopover: React.FunctionComponent<SignalPopoverProps> = ({
   onPopoverOpenChange: _onPopoverOpenChange,
 }) => {
   const hooks = useContext(TrackingHooksContext);
-  const [cueOpen, setCueOpen] = useState(false);
   const darkMode = useDarkMode(_darkMode);
   const [triggerVisible, setTriggerVisible] = useState(true);
   const [popoverOpen, setPopoverOpen] = useState(false);
-  const [hoverProps, isHovered] = useHoverState();
+  const [hoverProps, isHovered, setHovered] = useHoverState();
   const [currentSignalIndex, setCurrentSignalIndex] = useState(0);
   const signals = Array.isArray(_signals) ? _signals : [_signals];
   const currentSignal = signals[currentSignalIndex];
   const multiSignals = signals.length > 1;
-  const isActive = cueOpen || isHovered || popoverOpen;
+  const isActive = isHovered || popoverOpen;
+
   const triggerRef = useRef<HTMLButtonElement>(null);
 
   // To make sure we are covering signals added to the signal popover during the
@@ -549,12 +552,20 @@ const SignalPopover: React.FunctionComponent<SignalPopoverProps> = ({
             description="Across Compass, you may now see icons like this to clue you in on potential areas of improvement for your data."
             buttonText="See insights in action"
             onPrimaryButtonClick={() => {
-              triggerRef.current?.click();
+              // Because the guide cue is currently in inactive state when this
+              // button is clicked, the popover position can be calculated
+              // incorrectly because the expand animation will be triggered at
+              // the same time as popover show animation. To work around that,
+              // we will first manually trigger hover state, wait for the transition
+              // duration, and only then will click the trigger to open the
+              // popup
+              setHovered(true);
+              setTimeout(() => {
+                rafraf(() => {
+                  triggerRef.current?.click();
+                });
+              }, TRANSITION_DURATION_MS);
             }}
-            // So that the insight badge can animate without messing the tooltip
-            // position
-            tooltipAlign="right"
-            onOpenChange={setCueOpen}
             trigger={({ ref: guideCueRef }) => {
               const props = mergeProps<HTMLButtonElement>(
                 hoverProps,

packages/compass-components/src/hooks/use-focus-hover.ts

@@ -57,7 +57,11 @@ export function useFocusState(): [
   return [mergedProps, focusStateRef.current, focusStateRef];
 }
 
-export function useHoverState(): [React.HTMLAttributes<HTMLElement>, boolean] {
+export function useHoverState(): [
+  React.HTMLAttributes<HTMLElement>,
+  boolean,
+  React.Dispatch<React.SetStateAction<boolean>>
+] {
   const [isHovered, setIsHovered] = useState(false);
   const hoverProps = {
     onMouseEnter() {
@@ -67,5 +71,5 @@ export function useHoverState(): [React.HTMLAttributes<HTMLElement>, boolean] {
       setIsHovered(false);
     },
   };
-  return [hoverProps, isHovered];
+  return [hoverProps, isHovered, setIsHovered];
 }

packages/compass-e2e-tests/helpers/selectors.ts

@@ -1129,3 +1129,7 @@ export const DuplicateViewModalConfirmButton = `${DuplicateViewModal} button[typ
 
 // Modify view
 export const ModifySourceBanner = '[data-testid="modify-source-banner"]';
+
+// Insights
+export const InsightIconButton = '[data-testid="insight-badge-button"]';
+export const InsightPopoverCard = '[data-testid="insight-signal-card"]';

packages/compass-e2e-tests/tests/collection-documents-tab.test.ts

@@ -562,4 +562,19 @@ FindIterable<Document> result = collection.find(filter);`);
     const noDocuments = await browser.$('.document-list-zero-state');
     await noDocuments.waitForDisplayed();
   });
+
+  it('shows insight for the unindexed query', async function () {
+    await browser.runFindOperation('Documents', '{ i: 35 }');
+    await browser.clickVisible(Selectors.InsightIconButton);
+    await browser.waitForAnimations(Selectors.InsightPopoverCard);
+    const unindexedQuerySignal = await browser.$(
+      'strong=Query executed without index'
+    );
+    // Looks redundant, but selector above can return a special webdriver
+    // non-existing element, so we try to get some text so that it will actually
+    // throw if not found in DOM
+    expect(await unindexedQuerySignal.getText()).to.eq(
+      'Query executed without index'
+    );
+  });
 });

packages/compass-preferences-model/src/feature-flags.ts

@@ -82,7 +82,7 @@ export const featureFlags: Required<{
   },
 
   showInsights: {
-    stage: 'development',
+    stage: 'released',
     description: {
       short: 'Show performance insights',
       long: 'Surface visual signals in the Compass interface to highlight potential performance issues and anti-patterns.',

packages/compass-query-bar/src/components/option-editor.tsx

@@ -54,8 +54,10 @@ const editorWithErrorStyles = css({
 
 const queryBarEditorOptionInsightsStyles = css({
   position: 'absolute',
-  // Horizontally the insight is in the middle of the first line of the editor
-  top: `calc((${spacing[4]}px - 18px) / 2)`,
+  // Horizontally the insight is in the middle of the first line of the editor:
+  // (input height - insight badge height) / 2 to get the empty space + 1px
+  // because top indicates where element starts, not where padding ends
+  top: `calc((${spacing[4]}px - 18px) / 2 + 1px)`,
   right: spacing[1],
 });