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

fix: table sorting during insight creation #11456

Merged
merged 5 commits into from
Aug 24, 2022
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
40 changes: 26 additions & 14 deletions frontend/src/lib/components/LemonTable/LemonTable.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import clsx from 'clsx'
import { useActions, useValues } from 'kea'
import { router } from 'kea-router'
import React, { HTMLProps, useCallback, useEffect, useMemo } from 'react'
import React, { HTMLProps, useCallback, useEffect, useMemo, useState } from 'react'
import { Tooltip } from '../Tooltip'
import { TableRow } from './TableRow'
import './LemonTable.scss'
Expand Down Expand Up @@ -62,6 +62,8 @@ export interface LemonTableProps<T extends Record<string, any>> {
sorting?: Sorting | null
/** Sorting change handler for controlled sort order. */
onSort?: (newSorting: Sorting | null) => void
/** Defaults to true. Used if you don't want to use the URL to store sort order **/
useURLForSorting?: boolean
/** How many skeleton rows should be used for the empty loading state. The default value is 1. */
loadingSkeletonRows?: number
/** What to show when there's no data. */
Expand Down Expand Up @@ -94,6 +96,7 @@ export function LemonTable<T extends Record<string, any>>({
defaultSorting = null,
sorting,
onSort,
useURLForSorting = true,
loadingSkeletonRows = 1,
emptyState,
nouns = ['entry', 'entries'],
Expand All @@ -108,19 +111,27 @@ export function LemonTable<T extends Record<string, any>>({
const { location, searchParams, hashParams } = useValues(router)
const { push } = useActions(router)

/** Replace the current browsing history item to change sorting */
// used when not using URL to store sorting
const [internalSorting, setInternalSorting] = useState<Sorting | null>(sorting || null)

/** update sorting and conditionally replace the current browsing history item */
const setLocalSorting = useCallback(
(newSorting: Sorting | null) =>
push(
location.pathname,
{
...searchParams,
[currentSortingParam]: newSorting
? `${newSorting.order === -1 ? '-' : ''}${newSorting.columnKey}`
: undefined,
},
hashParams
),
(newSorting: Sorting | null) => {
setInternalSorting(newSorting)
onSort?.(newSorting)
if (useURLForSorting) {
return push(
location.pathname,
{
...searchParams,
[currentSortingParam]: newSorting
? `${newSorting.order === -1 ? '-' : ''}${newSorting.columnKey}`
: undefined,
},
hashParams
)
}
},
[location, searchParams, hashParams, push]
)

Expand All @@ -140,6 +151,7 @@ export function LemonTable<T extends Record<string, any>>({
/** Sorting. */
const currentSorting =
sorting ||
internalSorting ||
(searchParams[currentSortingParam]
? searchParams[currentSortingParam].startsWith('-')
? {
Expand Down Expand Up @@ -248,8 +260,8 @@ export function LemonTable<T extends Record<string, any>>({
determineColumnKey(column, 'sorting'),
disableSortingCancellation
)

setLocalSorting(nextSorting)
onSort?.(nextSorting)
}
: undefined
}
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/scenes/insights/InsightContainer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ export function InsightContainer(
) {
return (
<>
<h2 style={{ margin: '1rem 0' }}>Detailed results</h2>
<h2 className="my-4 mx-0">Detailed results</h2>
<FunnelStepsTable />
</>
)
Expand All @@ -137,7 +137,7 @@ export function InsightContainer(
return (
<>
{exporterResourceParams && (
<div className="flex items-center justify-between" style={{ margin: '1rem 0' }}>
<div className="flex items-center justify-between my-4 mx-0">
<h2>Detailed results</h2>
<Tooltip title="Export this table in CSV format" placement="left">
<ExportButton
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { trendsLogic } from 'scenes/trends/trendsLogic'
import { LemonCheckbox } from 'lib/components/LemonCheckbox'
import { getSeriesColor } from 'lib/colors'
import { cohortsModel } from '~/models/cohortsModel'
import { ChartDisplayType, IntervalType, TrendResult } from '~/types'
import { ChartDisplayType, IntervalType, ItemMode, TrendResult } from '~/types'
import { average, median, capitalizeFirstLetter } from 'lib/utils'
import { InsightLabel } from 'lib/components/InsightLabel'
import { PropertyKeyInfo } from 'lib/components/PropertyKeyInfo'
Expand All @@ -28,6 +28,7 @@ import { NON_TIME_SERIES_DISPLAY_TYPES } from 'lib/constants'
import { propertyDefinitionsModel } from '~/models/propertyDefinitionsModel'
import { formatAggregationValue, formatBreakdownLabel } from 'scenes/insights/utils'
import { formatAggregationAxisValue } from 'scenes/insights/aggregationAxisFormat'
import { insightSceneLogic } from 'scenes/insights/insightSceneLogic'

interface InsightsTableProps {
/** Whether this is just a legend instead of standalone insight viz. Default: false. */
Expand Down Expand Up @@ -72,6 +73,7 @@ export function InsightsTable({
isMainInsightView = false,
}: InsightsTableProps): JSX.Element | null {
const { insightProps, isViewedOnDashboard, insight } = useValues(insightLogic)
const { insightMode } = useValues(insightSceneLogic)
const { indexedResults, hiddenLegendKeys, filters, resultsLoading } = useValues(trendsLogic(insightProps))
const { toggleVisibility, setFilters } = useActions(trendsLogic(insightProps))
const { cohorts } = useValues(cohortsModel)
Expand Down Expand Up @@ -306,6 +308,8 @@ export function InsightsTable({
})
}

const useURLForSorting = insightMode !== ItemMode.Edit

return (
<LemonTable
id={isViewedOnDashboard ? insight.short_id : undefined}
Expand All @@ -318,6 +322,7 @@ export function InsightsTable({
emptyState="No insight results"
data-attr="insights-table-graph"
className="insights-table"
useURLForSorting={useURLForSorting}
/>
)
}
Expand Down