Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(product-analytics): lineGraphLogic.ts to tooltip-data.ts #19815

Merged
merged 3 commits into from
Jan 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ module.exports = {
// let's save the humans time and let the machines do the work
// "never" means if the prop does not need the curly braces, they will be removed/errored
// see https://github.com/jsx-eslint/eslint-plugin-react/blob/master/docs/rules/jsx-curly-brace-presence.md
"react/jsx-curly-brace-presence": ['error', { "props": "never", "children": "never", "propElementValues": "always" }],
'react/jsx-curly-brace-presence': ['error', { props: 'never', children: 'never', propElementValues: 'always' }],
'no-console': ['error', { allow: ['warn', 'error'] }],
'no-debugger': 'error',
'no-only-tests/no-only-tests': 'error',
Expand Down
2 changes: 1 addition & 1 deletion .storybook/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ const config: StorybookConfig = {

framework: {
name: '@storybook/react-webpack5',
options: { builder: { useSWC: true } }
options: { builder: { useSWC: true } },
},

docs: {
Expand Down
4 changes: 2 additions & 2 deletions .stylelintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ module.exports = {
'scss/at-rule-no-unknown': [
true,
{
'ignoreAtRules': ['tailwind']
}
ignoreAtRules: ['tailwind'],
},
],
'scss/operator-no-newline-after': null, // Doesn't always play well with prettier
'scss/at-extend-no-missing-placeholder': null,
Expand Down
26 changes: 13 additions & 13 deletions bin/build-schema.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -2,27 +2,27 @@

// replaces ts-json-schema-generator -f tsconfig.json --path 'frontend/src/queries/schema.ts' --no-type-check > frontend/src/queries/schema.json

import fs from "fs";
import stableStringify from "safe-stable-stringify";
import tsj from "ts-json-schema-generator";
import fs from 'fs'
import stableStringify from 'safe-stable-stringify'
import tsj from 'ts-json-schema-generator'

/** @type {import('ts-json-schema-generator/dist/src/Config').Config} */
const config = {
...tsj.DEFAULT_CONFIG,
path: "frontend/src/queries/schema.ts",
tsconfig: "tsconfig.json",
discriminatorType: "open-api",
path: 'frontend/src/queries/schema.ts',
tsconfig: 'tsconfig.json',
discriminatorType: 'open-api',
skipTypeCheck: true,
};
}

const output_path = "frontend/src/queries/schema.json";
const output_path = 'frontend/src/queries/schema.json'

const schema = tsj.createGenerator(config).createSchema(config.type);
const stringify = config.sortProps ? stableStringify : JSON.stringify;
const schemaString = (config.minify ? stringify(schema) : stringify(schema, null, 2));
const schema = tsj.createGenerator(config).createSchema(config.type)
const stringify = config.sortProps ? stableStringify : JSON.stringify
const schemaString = config.minify ? stringify(schema) : stringify(schema, null, 2)

fs.writeFile(output_path, schemaString, (err) => {
if (err) {
throw err;
throw err
}
});
})
12 changes: 11 additions & 1 deletion frontend/src/queries/nodes/DataTable/renderColumn.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,17 @@ export function renderColumn(
object[value[i]] = value[i + 1]
}
if ('results' in object && Array.isArray(object.results)) {
return <Sparkline data={object.results} />
// TODO: If results aren't an array of numbers, show a helpful message on using sparkline()
return (
<Sparkline
data={[
{
name: key.includes('__hogql_chart_type') ? 'Data' : key,
values: object.results.map((v: any) => Number(v)),
},
]}
/>
)
}
}

Expand Down
4 changes: 1 addition & 3 deletions frontend/src/scenes/insights/views/LineGraph/LineGraph.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ import { insightLogic } from 'scenes/insights/insightLogic'
import { InsightTooltip } from 'scenes/insights/InsightTooltip/InsightTooltip'
import { TooltipConfig } from 'scenes/insights/InsightTooltip/insightTooltipUtils'
import { insightVizDataLogic } from 'scenes/insights/insightVizDataLogic'
import { lineGraphLogic } from 'scenes/insights/views/LineGraph/lineGraphLogic'
import { PieChart } from 'scenes/insights/views/LineGraph/PieChart'
import { createTooltipData } from 'scenes/insights/views/LineGraph/tooltip-data'

import { ErrorBoundary } from '~/layout/ErrorBoundary'
import { themeLogic } from '~/layout/navigation-3000/themeLogic'
Expand Down Expand Up @@ -278,8 +278,6 @@ export function LineGraph_({
const { insightProps, insight } = useValues(insightLogic)
const { timezone, isTrends } = useValues(insightVizDataLogic(insightProps))

const { createTooltipData } = useValues(lineGraphLogic)

const canvasRef = useRef<HTMLCanvasElement | null>(null)
const [myLineChart, setMyLineChart] = useState<Chart<ChartType, any, string>>()

Expand Down
3 changes: 1 addition & 2 deletions frontend/src/scenes/insights/views/LineGraph/PieChart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import {
onChartClick,
onChartHover,
} from 'scenes/insights/views/LineGraph/LineGraph'
import { lineGraphLogic } from 'scenes/insights/views/LineGraph/lineGraphLogic'
import { createTooltipData } from 'scenes/insights/views/LineGraph/tooltip-data'

import { areObjectValuesEmpty } from '~/lib/utils'
import { groupsModel } from '~/models/groupsModel'
Expand Down Expand Up @@ -84,7 +84,6 @@ export function PieChart({

let datasets = _datasets

const { createTooltipData } = useValues(lineGraphLogic)
const { aggregationLabel } = useValues(groupsModel)
const { highlightSeries } = useActions(insightLogic)
const canvasRef = useRef<HTMLCanvasElement | null>(null)
Expand Down
57 changes: 0 additions & 57 deletions frontend/src/scenes/insights/views/LineGraph/lineGraphLogic.ts

This file was deleted.

47 changes: 47 additions & 0 deletions frontend/src/scenes/insights/views/LineGraph/tooltip-data.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import { TooltipItem } from 'chart.js'
import { SeriesDatum } from 'scenes/insights/InsightTooltip/insightTooltipUtils'

import { GraphDataset } from '~/types'

export function createTooltipData(
tooltipDataPoints: TooltipItem<any>[],
filterFn?: (s: SeriesDatum) => boolean
): SeriesDatum[] {
if (!tooltipDataPoints) {
return []
}
let data = tooltipDataPoints
.map((dp, idx) => {
const pointDataset = (dp?.dataset ?? {}) as GraphDataset
return {
id: idx,
dataIndex: dp.dataIndex,
datasetIndex: dp.datasetIndex,
dotted: !!pointDataset?.dotted,
breakdown_value:
pointDataset?.breakdown_value ?? pointDataset?.breakdownValues?.[dp.dataIndex] ?? undefined,
compare_label: pointDataset?.compare_label ?? pointDataset?.compareLabels?.[dp.dataIndex] ?? undefined,
action: pointDataset?.action ?? pointDataset?.actions?.[dp.dataIndex] ?? undefined,
label: pointDataset?.label ?? pointDataset.labels?.[dp.dataIndex] ?? undefined,
color: Array.isArray(pointDataset.borderColor)
? pointDataset.borderColor?.[dp.dataIndex]
: pointDataset.borderColor,
count: pointDataset?.data?.[dp.dataIndex] || 0,
filter: pointDataset?.filter ?? {},
}
})
.sort((a, b) => {
// Sort by descending order and fallback on alphabetic sort
return (
b.count - a.count ||
(a.label === undefined || b.label === undefined ? -1 : a.label.localeCompare(b.label))
)
})
if (filterFn) {
data = data.filter(filterFn)
}
return data.map((s, id) => ({
...s,
id,
}))
}
Loading