Code Insights: Support color for compute powered insight (#40038)

vovakulikov · web-flow · commit bd796dc9784a · 2022-08-11T12:43:32.000+03:00
* Adjust compute-insight creation UI

* Use only one colour for compute-powered insight on the dashboard page

* Handle no data state for the compute insight

* Refactor backend insight and chart card

* Disable stylelint for the hack with self selector

* Fix copy for compute powered insight card
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -29,6 +29,7 @@ All notable changes to Sourcegraph are documented in this file.
 - **IMPORTANT: Search queries with patterns surrounded by** `/.../` **will now be interpreted as regular expressions.** Existing search links or code monitors are unaffected. In the rare event where older links rely on the literal meaning of `/.../`, the string will be automatically quoted it in a `content` filter, preserving the original meaning. If you happen to use an existing older link and want `/.../` to work as a regular expression, add `patterntype:standard` to the query. New queries and code monitors will interpret `/.../` as regular expressions. [#38141](https://github.com/sourcegraph/sourcegraph/pull/38141).
 - The password policy has been updated and is now part of the standard featureset configurable by site-admins. [#39213](https://github.com/sourcegraph/sourcegraph/pull/39213).
 - Replaced the `ALLOW_DECRYPT_MIGRATION` envvar with `ALLOW_DECRYPTION`. See [updated documentation](https://docs.sourcegraph.com/admin/config/encryption). [#39984](https://github.com/sourcegraph/sourcegraph/pull/39984)
+- Compute-powered insight now supports only one series custom colors for compute series bars [40038](https://github.com/sourcegraph/sourcegraph/pull/40038)
 
 ### Fixed
 
diff --git a/client/web/src/enterprise/insights/components/creation-ui/form-series/FormSeries.tsx b/client/web/src/enterprise/insights/components/creation-ui/form-series/FormSeries.tsx
@@ -28,11 +28,11 @@ export interface FormSeriesProps {
     queryFieldDescription?: ReactNode
 
     /**
-     * This prop hides color picker from the series form. This field is needed for
-     * compute powered insight creation UI, see https://github.com/sourcegraph/sourcegraph/issues/38832
-     * for more details compute doesn't have series colors
+     * For the compute-powered insight we do not support multi series, in order to enforce it
+     * we need to hide this functionality by hiding add new series button.
+     * More context in this issue https://github.com/sourcegraph/sourcegraph/issues/38832
      */
-    showColorPicker?: boolean
+    hasAddNewSeriesButton?: boolean
 }
 
 export const FormSeries: FC<FormSeriesProps> = props => {
@@ -41,7 +41,7 @@ export const FormSeries: FC<FormSeriesProps> = props => {
         showValidationErrorsOnMount,
         repositories,
         queryFieldDescription,
-        showColorPicker = true,
+        hasAddNewSeriesButton = true,
     } = props
 
     const { licensed } = useUiFeatures()
@@ -60,7 +60,6 @@ export const FormSeries: FC<FormSeriesProps> = props => {
                         autofocus={line.autofocus}
                         repositories={repositories}
                         queryFieldDescription={queryFieldDescription}
-                        showColorPicker={showColorPicker}
                         className={classNames('p-3', styles.formSeriesItem)}
                         onSubmit={editCommit}
                         onCancel={() => cancelEdit(line.id)}
@@ -74,7 +73,6 @@ export const FormSeries: FC<FormSeriesProps> = props => {
                             onEdit={() => editRequest(line.id)}
                             onRemove={() => deleteSeries(line.id)}
                             className={styles.formSeriesItem}
-                            showColor={showColorPicker}
                             {...line}
                         />
                     )
@@ -85,16 +83,18 @@ export const FormSeries: FC<FormSeriesProps> = props => {
                 <LimitedAccessLabel message="Unlock Code Insights for unlimited data series" className="mx-auto my-3" />
             )}
 
-            <Button
-                data-testid="add-series-button"
-                type="button"
-                onClick={() => editRequest()}
-                variant="link"
-                disabled={!licensed ? series.length >= 10 : false}
-                className={classNames(styles.formSeriesItem, styles.formSeriesAddButton, 'p-3')}
-            >
-                + Add another data series
-            </Button>
+            {hasAddNewSeriesButton && (
+                <Button
+                    data-testid="add-series-button"
+                    type="button"
+                    onClick={() => editRequest()}
+                    variant="link"
+                    disabled={!licensed ? series.length >= 10 : false}
+                    className={classNames(styles.formSeriesItem, styles.formSeriesAddButton, 'p-3')}
+                >
+                    + Add another data series
+                </Button>
+            )}
         </ul>
     )
 }
diff --git a/client/web/src/enterprise/insights/components/creation-ui/form-series/components/form-series-input/FormSeriesInput.tsx b/client/web/src/enterprise/insights/components/creation-ui/form-series/components/form-series-input/FormSeriesInput.tsx
@@ -36,13 +36,6 @@ interface FormSeriesInputProps {
      */
     queryFieldDescription?: ReactNode
 
-    /**
-     * This prop hides color picker from the series form. This field is needed for
-     * compute powered insight creation UI, see https://github.com/sourcegraph/sourcegraph/issues/38832
-     * for more details whe compute doesn't have series colors
-     */
-    showColorPicker: boolean
-
     /** Enable autofocus behavior of the first input element of series form. */
     autofocus?: boolean
 
@@ -72,7 +65,6 @@ export const FormSeriesInput: FC<FormSeriesInputProps> = props => {
         autofocus = true,
         repositories,
         queryFieldDescription,
-        showColorPicker,
         onCancel = noop,
         onSubmit = noop,
         onChange = noop,
@@ -156,15 +148,13 @@ export const FormSeriesInput: FC<FormSeriesInputProps> = props => {
                 {...getDefaultInputProps(queryField)}
             />
 
-            {showColorPicker && (
-                <FormColorInput
-                    name={`color group of ${index} series`}
-                    title="Color"
-                    className="mt-4"
-                    value={colorField.input.value}
-                    onChange={colorField.input.onChange}
-                />
-            )}
+            <FormColorInput
+                name={`color group of ${index} series`}
+                title="Color"
+                className="mt-4"
+                value={colorField.input.value}
+                onChange={colorField.input.onChange}
+            />
 
             <div className="mt-4">
                 <Button
diff --git a/client/web/src/enterprise/insights/components/creation-ui/form-series/components/series-card/SeriesCard.tsx b/client/web/src/enterprise/insights/components/creation-ui/form-series/components/series-card/SeriesCard.tsx
@@ -18,13 +18,6 @@ interface SeriesCardProps {
     /** Color value of series. */
     stroke?: string
 
-    /**
-     * This prop hides color picker from the series form. This field is needed for
-     * compute powered insight creation UI, see https://github.com/sourcegraph/sourcegraph/issues/38832
-     * for more details whe compute doesn't have series colors
-     */
-    showColor?: boolean
-
     /** Custom class name for root button element. */
     className?: string
     /** Edit handler. */
@@ -37,16 +30,7 @@ interface SeriesCardProps {
  * Renders series card component, visual list item of series (name, color, query)
  * */
 export function SeriesCard(props: SeriesCardProps): ReactElement {
-    const {
-        disabled,
-        name,
-        query,
-        stroke: color = DEFAULT_DATA_SERIES_COLOR,
-        showColor = true,
-        className,
-        onEdit,
-        onRemove,
-    } = props
+    const { disabled, name, query, stroke: color = DEFAULT_DATA_SERIES_COLOR, className, onEdit, onRemove } = props
 
     return (
         <Card
@@ -58,14 +42,12 @@ export function SeriesCard(props: SeriesCardProps): ReactElement {
         >
             <div className={styles.cardInfo}>
                 <div className={classNames('mb-1 ', styles.cardTitle)}>
-                    {showColor && (
-                        <div
-                            data-testid="series-color-mark"
-                            /* eslint-disable-next-line react/forbid-dom-props */
-                            style={{ color: disabled ? 'var(--icon-muted)' : color }}
-                            className={styles.cardColorMark}
-                        />
-                    )}
+                    <div
+                        data-testid="series-color-mark"
+                        /* eslint-disable-next-line react/forbid-dom-props */
+                        style={{ color: disabled ? 'var(--icon-muted)' : color }}
+                        className={styles.cardColorMark}
+                    />
                     <span
                         data-testid="series-name"
                         title={name}
diff --git a/client/web/src/enterprise/insights/components/insights-view-grid/components/backend-insight/BackendInsight.tsx b/client/web/src/enterprise/insights/components/insights-view-grid/components/backend-insight/BackendInsight.tsx
@@ -32,8 +32,8 @@ import {
     DrillDownFiltersPopover,
     DrillDownInsightCreationFormValues,
     BackendInsightChart,
+    parseSeriesLimit,
 } from './components'
-import { parseSeriesLimit } from './components/drill-down-filters-panel/drill-down-filters/utils'
 
 import styles from './BackendInsight.module.scss'
 
@@ -67,14 +67,9 @@ export const BackendInsightView: React.FunctionComponent<React.PropsWithChildren
     const mergedInsightCardReference = useMergeRefs([insightCardReference, innerRef])
     const { wasEverVisible, isVisible } = useVisibility(insightCardReference)
 
-    // Use deep copy check in case if a setting subject has re-created copy of
-    // the insight config with same structure and values. To avoid insight data
-    // re-fetching.
-    const cachedInsight = useDeepMemo(insight)
-
     // Original insight filters values that are stored in setting subject with insight
     // configuration object, They are updated  whenever the user clicks update/save button
-    const [originalInsightFilters, setOriginalInsightFilters] = useState(cachedInsight.filters)
+    const [originalInsightFilters, setOriginalInsightFilters] = useState(insight.filters)
 
     // Live valid filters from filter form. They are updated whenever the user is changing
     // filter value in filters fields.
@@ -181,8 +176,6 @@ export const BackendInsightView: React.FunctionComponent<React.PropsWithChildren
         insightType: getTrackingTypeByInsightType(insight.type),
     })
 
-    const shareableUrl = `${window.location.origin}/insights/insight/${insight.id}`
-
     return (
         <InsightCard
             {...otherProps}
@@ -194,7 +187,11 @@ export const BackendInsightView: React.FunctionComponent<React.PropsWithChildren
         >
             <InsightCardHeader
                 title={
-                    <Link to={shareableUrl} target="_blank" rel="noopener noreferrer">
+                    <Link
+                        to={`${window.location.origin}/insights/insight/${insight.id}`}
+                        target="_blank"
+                        rel="noopener noreferrer"
+                    >
                         {insight.title}
                     </Link>
                 }
diff --git a/client/web/src/enterprise/insights/components/insights-view-grid/components/backend-insight/components/backend-insight-chart/BackendInsightChart.module.scss b/client/web/src/enterprise/insights/components/insights-view-grid/components/backend-insight/components/backend-insight-chart/BackendInsightChart.module.scss
@@ -7,19 +7,31 @@
     grid-template-columns: auto minmax(10rem, 30%);
     grid-template-rows: auto 1fr;
     gap: 0.75rem;
+
     grid-template-areas:
         'chart chart'
-        'chart chart'
-        'legend legend';
+        'chart chart';
 
-    &--horizontal {
+    // Hack for generating a proper scoped css nested class
+    // see https://css-tricks.com/using-sass-control-scope-bem-naming/
+    $self: &;
+
+    // stylelint-disable-next-line selector-class-pattern
+    &--withLegend {
         grid-template-areas:
-            'chart legend'
-            'chart legend';
+            'chart chart'
+            'chart chart'
+            'legend legend';
+
+        &#{ $self }--horizontal {
+            grid-template-areas:
+                'chart legend'
+                'chart legend';
 
-        .legend-list {
-            flex-wrap: nowrap;
-            flex-direction: column;
+            .legend-list {
+                flex-wrap: nowrap;
+                flex-direction: column;
+            }
         }
     }
 }
diff --git a/client/web/src/enterprise/insights/components/insights-view-grid/components/backend-insight/components/backend-insight-chart/BackendInsightChart.tsx b/client/web/src/enterprise/insights/components/insights-view-grid/components/backend-insight/components/backend-insight-chart/BackendInsightChart.tsx
@@ -4,12 +4,10 @@ import { ParentSize } from '@visx/responsive'
 import classNames from 'classnames'
 import useResizeObserver from 'use-resize-observer'
 
-import { useDebounce } from '@sourcegraph/wildcard'
-
 import { getLineColor, LegendItem, LegendList, ScrollBox, Series } from '../../../../../../../../charts'
 import { BarChart } from '../../../../../../../../charts/components/bar-chart/BarChart'
 import { UseSeriesToggleReturn } from '../../../../../../../../insights/utils/use-series-toggle'
-import { BackendInsightData, CategoricalChartContent, InsightContent } from '../../../../../../core'
+import { BackendInsightData, InsightContent } from '../../../../../../core'
 import { InsightContentType } from '../../../../../../core/types/insight/common'
 import { SeriesBasedChartTypes, SeriesChart } from '../../../../../views'
 import { BackendAlertOverlay } from '../backend-insight-alerts/BackendInsightAlerts'
@@ -51,17 +49,24 @@ interface BackendInsightChartProps<Datum> extends BackendInsightData {
 
 export function BackendInsightChart<Datum>(props: BackendInsightChartProps<Datum>): React.ReactElement {
     const { locked, isFetchingHistoricalData, data, zeroYAxisMin, className, onDatumClick, seriesToggleState } = props
-    const { ref, width = 0 } = useDebounce(useResizeObserver(), 100)
+    const { ref, width = 0 } = useResizeObserver()
     const { setHoveredId } = seriesToggleState
 
+    const isEmptyDataset = useMemo(() => hasNoData(data), [data])
+
     const hasViewManySeries = isManyKeysInsight(data)
     const hasEnoughXSpace = width >= MINIMAL_HORIZONTAL_LAYOUT_WIDTH
-
     const isHorizontalMode = hasViewManySeries && hasEnoughXSpace
-    const isEmptyDataset = useMemo(() => hasNoData(data), [data])
+    const isSeriesLikeInsight = data.type === InsightContentType.Series
 
     return (
-        <div ref={ref} className={classNames(className, styles.root, { [styles.rootHorizontal]: isHorizontalMode })}>
+        <div
+            ref={ref}
+            className={classNames(className, styles.root, {
+                [styles.rootHorizontal]: isHorizontalMode,
+                [styles.rootWithLegend]: isSeriesLikeInsight,
+            })}
+        >
             {width && (
                 <>
                     <ParentSize
@@ -96,13 +101,11 @@ export function BackendInsightChart<Datum>(props: BackendInsightChartProps<Datum
                         )}
                     </ParentSize>
 
-                    <ScrollBox className={styles.legendListContainer} onMouseLeave={() => setHoveredId(undefined)}>
-                        {data.type === InsightContentType.Series ? (
+                    {isSeriesLikeInsight && (
+                        <ScrollBox className={styles.legendListContainer} onMouseLeave={() => setHoveredId(undefined)}>
                             <SeriesLegends series={data.content.series} seriesToggleState={seriesToggleState} />
-                        ) : (
-                            <CategoricalLegends data={data.content} />
-                        )}
-                    </ScrollBox>
+                        </ScrollBox>
+                    )}
                 </>
             )}
         </div>
@@ -165,26 +168,3 @@ const SeriesLegends: FC<SeriesLegendsProps> = props => {
         </LegendList>
     )
 }
-
-interface CategoricalLegendsProps {
-    data: CategoricalChartContent<any>
-}
-
-const CategoricalLegends: FC<CategoricalLegendsProps> = props => {
-    const { data } = props
-
-    return (
-        <LegendList className={styles.legendList}>
-            {data.data.map(item => (
-                <LegendItem
-                    key={item.id as string}
-                    color={data.getDatumColor(item) ?? 'gray'}
-                    name={data.getDatumName(item)}
-                    className={styles.legendListItem}
-                    // prevent accidental dragging events
-                    onMouseDown={event => event.stopPropagation()}
-                />
-            ))}
-        </LegendList>
-    )
-}
diff --git a/client/web/src/enterprise/insights/components/modals/ShareLinkModal/ShareLinkModal.tsx b/client/web/src/enterprise/insights/components/modals/ShareLinkModal/ShareLinkModal.tsx
@@ -17,7 +17,7 @@ import {
     isOrganizationOwner,
     isVirtualDashboard,
 } from '../../../core'
-import { useCopyURLHandler } from '../../../hooks/use-copy-url-handler'
+import { useCopyURLHandler } from '../../../hooks'
 
 import { decodeDashboardIds, GET_SHARABLE_INSIGHT_INFO_GQL } from './get-sharable-insight-info'
 
diff --git a/client/web/src/enterprise/insights/core/backend/gql-backend/methods/get-backend-insight-data/deserializators.ts b/client/web/src/enterprise/insights/core/backend/gql-backend/methods/get-backend-insight-data/deserializators.ts
@@ -2,7 +2,7 @@ import { InsightDataNode } from '../../../../../../../graphql-operations'
 import { BackendInsight, isComputeInsight } from '../../../../types'
 import { InsightContentType } from '../../../../types/insight/common'
 import { BackendInsightData } from '../../../code-insights-backend-types'
-import { createCategoricalChart } from '../../../utils/create-categorical-content'
+import { createComputeCategoricalChart } from '../../../utils/create-categorical-content'
 import { createLineChartContent } from '../../../utils/create-line-chart-content'
 
 export const MAX_NUMBER_OF_SERIES = 20
@@ -21,7 +21,7 @@ export const createBackendInsightData = (insight: BackendInsight, response: Insi
             isFetchingHistoricalData: isFetchingHistoricalData || seriesData.some(series => !series.label),
             data: {
                 type: InsightContentType.Categorical,
-                content: createCategoricalChart(seriesData),
+                content: createComputeCategoricalChart(insight, seriesData),
             },
         }
     }
diff --git a/client/web/src/enterprise/insights/core/backend/utils/create-categorical-content.ts b/client/web/src/enterprise/insights/core/backend/utils/create-categorical-content.ts
diff --git a/client/web/src/enterprise/insights/pages/insights/creation/compute/components/ComputeInsightCreationContent.tsx b/client/web/src/enterprise/insights/pages/insights/creation/compute/components/ComputeInsightCreationContent.tsx
diff --git a/client/web/src/enterprise/insights/pages/insights/creation/compute/components/ComputeLivePreview.tsx b/client/web/src/enterprise/insights/pages/insights/creation/compute/components/ComputeLivePreview.tsx
diff --git a/client/web/src/enterprise/insights/pages/insights/creation/intro/cards/InsightCards.tsx b/client/web/src/enterprise/insights/pages/insights/creation/intro/cards/InsightCards.tsx
