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

Sessions Table bug fixes #4963

Closed
wants to merge 17 commits into from
Closed
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
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ module.exports = {
'@typescript-eslint/no-inferrable-types': 'off',
'@typescript-eslint/ban-ts-comment': 'off',
'no-shadow': 'error',
'@typescript-eslint/no-non-null-assertion': 'error',
curly: 'error',
},
overrides: [
Expand Down
11 changes: 11 additions & 0 deletions cypress/integration/sessions.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,15 @@ describe('Sessions', () => {
cy.get('h1').should('contain', 'Sessions')
cy.get('[data-attr=sessions-table]').should('exist')
})

it('Sessions Table highlights matches', () => {
// Add pageview filter
cy.get('[data-attr=sessions-filter-open]').click()
cy.get('.ant-input').type('Pageview')
cy.get('.ant-list-item').contains('Pageview').click()

cy.get('[data-attr=sessions-apply-filters]').click()
cy.get('.ant-table-row-expand-icon-cell').first().click()
cy.get('[data-attr=sessions-table]').find('.sessions-event-highlighted').its('length').should('be.gt', 0)
})
})
40 changes: 40 additions & 0 deletions frontend/src/lib/components/ExpandIcon.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import React from 'react'
import clsx from 'clsx'

// Imitates how Antd renders the expand icon
// https://github.com/ant-design/ant-design/blob/master/components/table/ExpandIcon.tsx

interface ExpandIconProps {
prefixCls: string
onExpand: (record: any, e: React.MouseEvent<HTMLElement>) => void
record: any
expanded: boolean
expandable: boolean
children?: JSX.Element
}

function ExpandIcon({ prefixCls, onExpand, record, expanded, expandable, children }: ExpandIconProps): JSX.Element {
const iconPrefix = `${prefixCls}-row-expand-icon`
return (
<div
style={{ display: 'flex', alignItems: 'center' }}
onClick={(e) => {
onExpand(record, e)
e.stopPropagation()
}}
>
<button
type="button"
className={clsx(iconPrefix, 'mr-05', {
[`${iconPrefix}-spaced`]: !expandable,
[`${iconPrefix}-expanded`]: expandable && expanded,
[`${iconPrefix}-collapsed`]: expandable && !expanded,
})}
aria-label={expanded ? 'Collapse row' : 'Expand row'}
/>
{children}
</div>
)
}

export default ExpandIcon
6 changes: 4 additions & 2 deletions frontend/src/lib/components/PropertyKeyInfo.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,7 @@ interface PropertyKeyInfoInterface {
style?: any
disablePopover?: boolean
disableIcon?: boolean
ellipsis?: boolean
}

export function PropertyKeyInfo({
Expand All @@ -509,6 +510,7 @@ export function PropertyKeyInfo({
style,
disablePopover = false,
disableIcon = false,
ellipsis = true,
}: PropertyKeyInfoInterface): JSX.Element {
value = `${value}` // convert to string
let data = null
Expand All @@ -522,14 +524,14 @@ export function PropertyKeyInfo({
}
} else {
return (
<Typography.Text ellipsis={true} style={{ color: 'inherit', maxWidth: 400, ...style }} title={value}>
<Typography.Text ellipsis={ellipsis} style={{ color: 'inherit', maxWidth: 400, ...style }} title={value}>
{value}
</Typography.Text>
)
}
if (disableIcon) {
return (
<Typography.Text ellipsis={true} style={{ color: 'inherit', maxWidth: 400 }} title={data.label}>
<Typography.Text ellipsis={ellipsis} style={{ color: 'inherit', maxWidth: 400 }} title={data.label}>
{data.label}
</Typography.Text>
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ interface VirtualTableHeaderProps<RecordType> {
handleResize: (index: number) => ResizeHandler
layoutEffect?: CallableFunction
minColumnWidth: number
expandable?: boolean
expandable?: Record<string, any>
}

function ResizableTitle({
Expand Down Expand Up @@ -73,7 +73,9 @@ function VirtualTableHeader<RecordType>({
useLayoutEffect(() => (typeof layoutEffect === 'function' ? layoutEffect() : undefined))
return (
<div className="resizable-virtual-table-header">
{expandable && <div className="left-spacer" style={{ width: ANTD_EXPAND_BUTTON_WIDTH }} />}
{!!expandable && (
<div className="left-spacer" style={{ width: expandable?.columnWidth || ANTD_EXPAND_BUTTON_WIDTH }} />
)}
{columns.map(({ title, width, widthConstraints }, index) => {
const minColumnWidth = widthConstraints?.length ? widthConstraints[0] : defaultMinColumnWidth
const maxColumnWidth = widthConstraints?.length ? widthConstraints[1] : Infinity
Expand Down
1 change: 0 additions & 1 deletion frontend/src/lib/components/ResizableTable/index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@

.left-spacer {
background: rgb(250, 250, 250);
border-right: 1px solid #f0f0f0;
border-bottom: 1px solid #f0f0f0;
flex-grow: 0;
flex-shrink: 0;
Expand Down
3 changes: 2 additions & 1 deletion frontend/src/lib/components/ResizableTable/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export interface InternalColumnType<RecordType> extends ResizableColumnType<Reco

export type ResizeHandler = Exclude<ResizableProps['onResize'], undefined>

// https://github.com/ant-design/ant-design/blob/4cdd24f4ec1ffb638175bb6c2dbb4fd7f103d60f/components/table/style/index.less#L422-L424
export const ANTD_EXPAND_BUTTON_WIDTH = 48

interface ResizableTableProps<RecordType> extends TableProps<RecordType> {
Expand Down Expand Up @@ -217,7 +218,7 @@ export function ResizableTable<RecordType extends Record<any, any> = any>({
handleResize={handleColumnResize}
layoutEffect={updateTableWidth}
minColumnWidth={minColumnWidth}
expandable={!!props.expandable}
expandable={props.expandable}
/>
)}
<Table
Expand Down
13 changes: 13 additions & 0 deletions frontend/src/lib/components/icons.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,19 @@ export function IconEvents(): JSX.Element {
)
}

export function IconEventsShort({ size = 32 }: { size: number }): JSX.Element {
return (
<svg width={size} height={size} viewBox={`0 0 32 32`} fill="none" xmlns="http://www.w3.org/2000/svg">
<path
fillRule="evenodd"
clipRule="evenodd"
d="M15.882 4L5.366 9.455l10.607 4.787 10.412-4.858L15.883 4zM5.333 17.038l4.79-2.475 5.785 2.496 5.496-2.514 4.948 2.433-10.412 4.857-10.607-4.797zm.314 7.23l4.226-2.192 6.024 2.578 6.14-2.829 4.63 2.369-10.429 4.86-10.59-4.786z"
fill="currentColor"
/>
</svg>
)
}

export function IconActions(): JSX.Element {
return (
<svg width="1em" height="1em" viewBox="0 0 32 32" fill="none" xmlns="http://www.w3.org/2000/svg">
Expand Down
4 changes: 4 additions & 0 deletions frontend/src/lib/utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ export function uuid(): string {
)
}

export function isObjectEmpty(obj: Record<string, any>): boolean {
return obj && Object.keys(obj).length === 0 && obj.constructor === Object
}

export function toParams(obj: Record<string, any>): string {
function handleVal(val: any): string {
if (dayjs.isDayjs(val)) {
Expand Down
24 changes: 11 additions & 13 deletions frontend/src/scenes/events/eventsTableLogic.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,16 +155,6 @@ export const eventsTableLogic = kea({
}),

selectors: ({ selectors, props }) => ({
propertiesForUrl: [
() => [selectors.properties],
(properties) => {
if (Object.keys(properties).length > 0) {
return { properties }
} else {
return ''
}
},
],
eventsFormatted: [
() => [selectors.events, selectors.newEvents],
(events, newEvents) => formatEvents(events, newEvents, props.apiUrl),
Expand All @@ -181,12 +171,19 @@ export const eventsTableLogic = kea({

actionToUrl: ({ values }) => ({
setProperties: () => {
return [router.values.location.pathname, values.propertiesForUrl, window.location.hash]
return [
router.values.location.pathname,
{
...router.values.searchParams,
properties: values.properties,
},
window.location.hash,
]
},
}),

urlToAction: ({ actions, values }) => ({
'*': (_, searchParams) => {
'*': (_, searchParams, hashParams) => {
try {
// if the url changed, but we are not anymore on the page we were at when the logic was mounted
if (router.values.location.pathname !== values.initialPathname) {
Expand All @@ -198,7 +195,8 @@ export const eventsTableLogic = kea({
return
}

if (!objectsEqual(searchParams.properties || {}, values.properties)) {
const isFirstRedirect = hashParams.backTo // first time we've navigated here from another page
if (!objectsEqual(searchParams.properties || {}, values.properties) || isFirstRedirect) {
actions.setProperties(searchParams.properties || {})
}
},
Expand Down
29 changes: 19 additions & 10 deletions frontend/src/scenes/persons/Person.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,18 @@ import { NewPropertyComponent } from './NewPropertyComponent'

import relativeTime from 'dayjs/plugin/relativeTime'
import { TZLabel } from 'lib/components/TimezoneAware'
import { PersonsTabType } from '~/types'
import { PageHeader } from 'lib/components/PageHeader'
dayjs.extend(relativeTime)

const { TabPane } = Tabs

export function Person(): JSX.Element {
const [activeTab, setActiveTab] = useState('events')
const [activeCardTab, setActiveCardTab] = useState('properties')
const [mergeModalOpen, setMergeModalOpen] = useState(false)

const { person, personLoading, deletedPersonLoading, hasNewKeys } = useValues(personsLogic)
const { deletePerson, setPerson, editProperty } = useActions(personsLogic)
const { person, personLoading, deletedPersonLoading, hasNewKeys, activeTab } = useValues(personsLogic)
const { deletePerson, setPerson, editProperty, navigateToTab } = useActions(personsLogic)

const ids = (
<Menu>
Expand All @@ -50,9 +51,10 @@ export function Person(): JSX.Element {
<Row gutter={16}>
<Col span={16}>
<Tabs
defaultActiveKey={activeTab}
defaultActiveKey={PersonsTabType.EVENTS}
activeKey={activeTab}
onChange={(tab) => {
setActiveTab(tab)
navigateToTab(tab as PersonsTabType)
}}
>
<TabPane tab={<span data-attr="persons-events-tab">Events</span>} key="events" />
Expand All @@ -66,11 +68,18 @@ export function Person(): JSX.Element {
fixedFilters={{ person_id: person.id }}
/>
) : (
<SessionsView
key={person.distinct_ids.join('__')} // force refresh if distinct_ids change
personIds={person.distinct_ids}
isPersonPage
/>
<>
<PageHeader
title="Sessions"
caption="Explore how events are being processed within sessions."
style={{ marginTop: 0 }}
/>
<SessionsView
key={person.distinct_ids.join('__')} // force refresh if distinct_ids change
personIds={person.distinct_ids}
isPersonPage
/>
</>
)}
</div>
)}
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/scenes/persons/Persons.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ export function Persons({ cohort }: { cohort: CohortType }): JSX.Element {
loadPrevious={() => loadPersons(persons.previous)}
loadNext={() => loadPersons(persons.next)}
allColumns
cohort={cohort}
backTo={cohort ? 'Cohort' : 'Persons'}
/>
</div>
</div>
Expand Down
43 changes: 28 additions & 15 deletions frontend/src/scenes/persons/PersonsTable.tsx
Original file line number Diff line number Diff line change
@@ -1,18 +1,19 @@
import React, { useRef } from 'react'
import { Button } from 'antd'
import { combineUrl } from 'kea-router'
import dayjs from 'dayjs'
import relativeTime from 'dayjs/plugin/relativeTime'
import { TZLabel } from 'lib/components/TimezoneAware'
import { Link } from 'lib/components/Link'
import { PropertiesTable } from 'lib/components/PropertiesTable'
import { CohortType, PersonType } from '~/types'
import { PersonsTabType, PersonType, SessionsPropertyFilter } from '~/types'
import { ArrowRightOutlined, LeftOutlined, RightOutlined } from '@ant-design/icons'
import './Persons.scss'
import { CopyToClipboardInline } from 'lib/components/CopyToClipboard'
import dayjs from 'dayjs'
import { midEllipsis } from 'lib/utils'
import { PersonHeader } from './PersonHeader'

import relativeTime from 'dayjs/plugin/relativeTime'
import { TZLabel } from 'lib/components/TimezoneAware'
import { ResizableColumnType, ResizableTable } from 'lib/components/ResizableTable'

dayjs.extend(relativeTime)

interface PersonsTableType {
Expand All @@ -23,7 +24,9 @@ interface PersonsTableType {
loadPrevious?: () => void
loadNext?: () => void
allColumns?: boolean // whether to show all columns or not
cohort?: CohortType
backTo?: string // text to display next to `back to` arrow. if "Insights," deep link to Persons > Sessions
sessionsFilters?: Partial<SessionsPropertyFilter>[] // sessions filters from trends graphs
date?: string
}

export function PersonsTable({
Expand All @@ -34,14 +37,20 @@ export function PersonsTable({
loadPrevious,
loadNext,
allColumns,
cohort,
backTo = 'Persons',
sessionsFilters = [],
date = undefined,
}: PersonsTableType): JSX.Element {
const linkToPerson = (person: PersonType): string => {
const backTo = cohort
? `#backTo=Cohorts&backToURL=${window.location.pathname}`
: `#backTo=Persons&backToURL=${window.location.pathname}`
return `/person/${encodeURIComponent(person.distinct_ids[0])}${backTo}`
}
const deepLinkToPersonSessions = (person: PersonType): string =>
combineUrl(
`/person/${encodeURIComponent(person.distinct_ids[0])}`,
{ filters: sessionsFilters, date },
{
backTo,
backToURL: window.location.pathname + window.location.search + window.location.hash,
activeTab: backTo === 'Insights' ? PersonsTabType.SESSIONS : PersonsTabType.EVENTS,
}
).url

const topRef = useRef<HTMLSpanElement>(null)

Expand All @@ -52,7 +61,7 @@ export function PersonsTable({
span: 6,
render: function Render(person: PersonType) {
return (
<Link to={linkToPerson(person)} data-attr="goto-person-email">
<Link to={deepLinkToPersonSessions(person)} data-attr="goto-person-email">
<PersonHeader person={person} />
</Link>
)
Expand Down Expand Up @@ -99,7 +108,11 @@ export function PersonsTable({
render: function Render(person: PersonType, ...[, index]: [PersonType, number]) {
return (
<>
<Link to={linkToPerson(person)} data-attr={`goto-person-arrow-${index}`} data-test-goto-person>
<Link
to={deepLinkToPersonSessions(person)}
data-attr={`goto-person-arrow-${index}`}
data-test-goto-person
>
<ArrowRightOutlined style={{ float: 'right' }} />
{allColumns ? ' view' : ''}
</Link>
Expand Down
Loading