-
Notifications
You must be signed in to change notification settings - Fork 22
feat(in-app-analytics): New rule hit table + Filters bar + data fetching #1150
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
Changes from 7 commits
d62de6b
0a3ba81
8e22cdf
49317a5
0446975
1039a91
108b18b
26d1d83
d94711a
8a2ca15
dcfa690
3b7fe5a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,114 @@ | ||
| import { Spinner } from '@app-builder/components/Spinner'; | ||
| import { type RuleHitTableResponse } from '@app-builder/models/analytics/rule-hit'; | ||
| import { formatNumber, useFormatLanguage } from '@app-builder/utils/format'; | ||
| import { createColumnHelper, getCoreRowModel } from '@tanstack/react-table'; | ||
| import { useMemo, useState } from 'react'; | ||
| import { useTranslation } from 'react-i18next'; | ||
| import { Table, useTable } from 'ui-design-system'; | ||
|
|
||
| export function RulesHit({ | ||
| data, | ||
| isLoading, | ||
| }: { | ||
| data: RuleHitTableResponse[]; | ||
| isLoading: boolean; | ||
| }) { | ||
| const { t } = useTranslation(['analytics']); | ||
| const language = useFormatLanguage(); | ||
| const [expanded, setExpanded] = useState(false); | ||
|
|
||
| const visibleData = useMemo(() => (expanded ? data : data.slice(0, 5)), [expanded, data]); | ||
|
|
||
| const columnHelper = createColumnHelper<RuleHitTableResponse>(); | ||
| const toPercent = (value: number) => | ||
| formatNumber(value > 1 ? value / 100 : value, { | ||
| language, | ||
| style: 'percent', | ||
| maximumFractionDigits: 1, | ||
| }); | ||
|
|
||
| const columns = useMemo( | ||
| () => [ | ||
| columnHelper.accessor((row) => row.ruleName, { | ||
| id: 'rule', | ||
| header: t('analytics:ruleshit.columns.rule'), | ||
| size: 220, | ||
| cell: ({ getValue }) => <span className="line-clamp-1">{getValue()}</span>, | ||
| }), | ||
| columnHelper.accessor((row) => row.hitCount, { | ||
| id: 'hitCount', | ||
| header: t('analytics:ruleshit.columns.hit_count'), | ||
| size: 100, | ||
| cell: ({ getValue }) => <span>{formatNumber(getValue(), { language })}</span>, | ||
| }), | ||
| columnHelper.accessor((row) => row.hitRatio, { | ||
| id: 'hitRatio', | ||
| header: t('analytics:ruleshit.columns.hit_ratio'), | ||
| size: 120, | ||
| cell: ({ getValue }) => <span>{toPercent(getValue())}</span>, | ||
| }), | ||
| columnHelper.accessor((row) => row.pivotCount, { | ||
| id: 'pivotCount', | ||
| header: t('analytics:ruleshit.columns.pivot_count'), | ||
| size: 140, | ||
| cell: ({ getValue }) => <span>{formatNumber(getValue(), { language })}</span>, | ||
| }), | ||
| columnHelper.accessor((row) => row.pivotRatio, { | ||
| id: 'pivotRatio', | ||
| header: t('analytics:ruleshit.columns.pivot_ratio'), | ||
| size: 160, | ||
| cell: ({ getValue }) => <span>{toPercent(getValue())}</span>, | ||
| }), | ||
| ], | ||
| [columnHelper, language, t], | ||
| ); | ||
|
|
||
| const { table, getBodyProps, rows, getContainerProps } = useTable({ | ||
| data: visibleData, | ||
| columns, | ||
| columnResizeMode: 'onChange', | ||
| getCoreRowModel: getCoreRowModel(), | ||
| enableSorting: false, | ||
| }); | ||
| return ( | ||
| <div className="mt-v2-xl"> | ||
| <div className="flex items-center justify-between"> | ||
| <h2 className="text-h2 font-semibold">{t('analytics:ruleshit.title')}</h2> | ||
| </div> | ||
|
|
||
| <div | ||
| aria-busy={isLoading} | ||
| className="bg-white border border-grey-90 rounded-lg p-v2-md shadow-sm mt-v2-sm relative" | ||
| > | ||
| {isLoading ? ( | ||
| <div className="absolute inset-0 z-10 flex items-center justify-center rounded-lg bg-grey-98/80 hover:bg-grey-95/80"> | ||
| <Spinner className="size-6" /> | ||
| </div> | ||
| ) : null} | ||
| <div className="flex w-full flex-col items-start gap-v2-md"> | ||
| <Table.Container {...getContainerProps()} className="bg-grey-100 w-full"> | ||
| <Table.Header headerGroups={table.getHeaderGroups()} /> | ||
| <Table.Body {...getBodyProps()}> | ||
| {rows.map((row) => ( | ||
| <Table.Row key={row.id} row={row} /> | ||
| ))} | ||
| {!expanded && data.length > 5 ? ( | ||
| <tr | ||
| className="even:bg-grey-98 h-12 hover:bg-purple-98 cursor-pointer" | ||
| onClick={() => setExpanded(true)} | ||
| > | ||
| <td | ||
| className="text-s w-full truncate px-4 font-medium text-purple-65" | ||
| colSpan={table.getHeaderGroups()[0]?.headers.length ?? 5} | ||
| > | ||
| See more + | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. translation to add
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
| </td> | ||
| </tr> | ||
| ) : null} | ||
| </Table.Body> | ||
| </Table.Container> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| import { useSearchParams } from '@remix-run/react'; | ||
| import { subDays, subMonths } from 'date-fns'; | ||
| import { useCallback, useMemo } from 'react'; | ||
| import { useCallback, useEffect, useState } from 'react'; | ||
|
|
||
| type StaticDateRange = { type: 'static'; startDate: string; endDate: string }; | ||
| type DynamicDateRange = { type: 'dynamic'; fromNow: string }; | ||
|
|
@@ -17,39 +17,58 @@ function getDefaultRange(): { start: string; end: string } { | |
| return { start: toIso(start), end: toIso(end) }; | ||
| } | ||
|
|
||
| function parseQ(qValue: string | null): { range: IsoRange; compareRange: IsoRange | null } { | ||
| if (qValue) { | ||
| try { | ||
| const obj = JSON.parse(atob(qValue)) as { | ||
| range?: { start?: string; end?: string } | null; | ||
| compareRange?: { start?: string; end?: string } | null; | ||
| }; | ||
| if (obj?.range?.start && obj?.range?.end) { | ||
| return { | ||
| range: { start: obj.range.start, end: obj.range.end }, | ||
| compareRange: | ||
| obj.compareRange?.start && obj.compareRange?.end | ||
| ? { start: obj.compareRange.start, end: obj.compareRange.end } | ||
| : null, | ||
| }; | ||
| } | ||
| } catch { | ||
| // ignore malformed q | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for convenience, add a console log ? (this is client side or server side ?)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will change it in another PR (work todo on this part) |
||
| } | ||
| } | ||
| const defaults = getDefaultRange(); | ||
| return { range: { start: defaults.start, end: defaults.end }, compareRange: null }; | ||
| } | ||
|
|
||
| export function useDateRangeSearchParams() { | ||
| const [searchParams, setSearchParams] = useSearchParams(); | ||
| const parseQ = useCallback( | ||
| (qValue: string | null): { range: IsoRange; compareRange: IsoRange | null } => { | ||
| if (qValue) { | ||
| try { | ||
| const obj = JSON.parse(atob(qValue)) as { | ||
| range?: { start?: string; end?: string } | null; | ||
| compareRange?: { start?: string; end?: string } | null; | ||
| }; | ||
| if (obj?.range?.start && obj?.range?.end) { | ||
| return { | ||
| range: { start: obj.range.start, end: obj.range.end }, | ||
| compareRange: | ||
| obj.compareRange?.start && obj.compareRange?.end | ||
| ? { start: obj.compareRange.start, end: obj.compareRange.end } | ||
| : null, | ||
| }; | ||
| } | ||
| } catch { | ||
| // ignore malformed q | ||
| } | ||
| const [dateRangeState, setDateRangeState] = useState(() => { | ||
| // Initialize state with parsed values | ||
| const qValue = searchParams.get('q'); | ||
| return parseQ(qValue); | ||
| }); | ||
|
|
||
| // Parse and update state only when the parsed values actually change | ||
| useEffect(() => { | ||
| const qValue = searchParams.get('q'); | ||
| const parsed = parseQ(qValue); | ||
|
|
||
| // Only update state if values actually changed | ||
| setDateRangeState((current) => { | ||
| if ( | ||
| current.range.start !== parsed.range.start || | ||
| current.range.end !== parsed.range.end || | ||
| current.compareRange?.start !== parsed.compareRange?.start || | ||
| current.compareRange?.end !== parsed.compareRange?.end | ||
| ) { | ||
| return parsed; | ||
| } | ||
| const defaults = getDefaultRange(); | ||
| return { range: { start: defaults.start, end: defaults.end }, compareRange: null }; | ||
| }, | ||
| [], | ||
| ); | ||
| return current; | ||
| }); | ||
| }, [searchParams, parseQ]); | ||
|
|
||
| const { range, compareRange } = useMemo( | ||
| () => parseQ(searchParams.get('q')), | ||
| [parseQ, searchParams], | ||
| ); | ||
| const { range, compareRange } = dateRangeState; | ||
|
|
||
| const computeDynamicRange = useCallback((fromNow: string): IsoRange => { | ||
| const now = new Date(); | ||
|
|
@@ -75,7 +94,7 @@ export function useDateRangeSearchParams() { | |
| params.delete('end'); | ||
| return params; | ||
| }, | ||
| [parseQ], | ||
| [], | ||
| ); | ||
|
|
||
| const setDateRangeFilter = useCallback( | ||
|
|
@@ -95,7 +114,7 @@ export function useDateRangeSearchParams() { | |
| const dynamicRange = computeDynamicRange(dateRange.fromNow); | ||
| return writeQ(prev, { range: dynamicRange, compareRange: undefined }); | ||
| }, | ||
| { replace: true }, | ||
| { replace: false }, | ||
| ); | ||
| }, | ||
| [computeDynamicRange, setSearchParams, writeQ], | ||
|
|
@@ -116,7 +135,7 @@ export function useDateRangeSearchParams() { | |
|
|
||
| return writeQ(prev, { compareRange: nextCompare }); | ||
| }, | ||
| { replace: true }, | ||
| { replace: false }, | ||
| ); | ||
| }, | ||
| [computeDynamicRange, setSearchParams, writeQ], | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,5 +4,11 @@ | |
| "decisions.tooltip.daily": "date: {{date}}", | ||
| "decisions.tooltip.monthly": "mois de {{date}}", | ||
| "decisions.tooltip.weekly": "semaine du {{date}} <Br/>(Semaine {{weekNumber}})", | ||
| "filters.add_compare_period": "Ajouter une période de comparaison" | ||
| "filters.add_compare_period": "Ajouter une période de comparaison", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We'll do a full review of translations, but at the end of the project I think (so this cycle) |
||
| "ruleshit.columns.hit_count": "# déclenchements", | ||
| "ruleshit.columns.hit_ratio": "% déclenchements", | ||
| "ruleshit.columns.pivot_count": "# pivots/utilisateurs distincts", | ||
| "ruleshit.columns.pivot_ratio": "% pivots/utilisateurs distincts", | ||
| "ruleshit.columns.rule": "Règle", | ||
| "ruleshit.title": "Décisions par règle" | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To make it simpler, I'd rather we just return the csv with total values, in every case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done