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

feat(insights): Add edit button for dashboard filters #21841

Merged
merged 23 commits into from
Apr 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
39a8d5c
Add edit switch to dashboard to make clear these are changing dashboard
webjunkie Apr 12, 2024
5e0b618
Add DashboardEditBar to encapsulate
webjunkie Apr 25, 2024
f5f9949
Update UI snapshots for `chromium` (1)
github-actions[bot] Apr 25, 2024
9d0aff7
Update UI snapshots for `chromium` (2)
github-actions[bot] Apr 25, 2024
e4bd5b2
Update UI snapshots for `chromium` (2)
github-actions[bot] Apr 25, 2024
495d876
Disable and do not hide filters
webjunkie Apr 25, 2024
39e96dc
Mark dashboard stale
webjunkie Apr 25, 2024
f5d4877
Update UI snapshots for `chromium` (1)
github-actions[bot] Apr 25, 2024
a743bd5
Fix taxonomicGroupTypes prop
webjunkie Apr 26, 2024
3f6c277
Update UI snapshots for `chromium` (2)
github-actions[bot] Apr 29, 2024
c8b172f
Update UI snapshots for `chromium` (2)
github-actions[bot] Apr 29, 2024
bc685a2
Merge branch 'master' into feature/dashboard-edit-switch
webjunkie Apr 30, 2024
9629517
Move logic inside DashboardEditBar
webjunkie Apr 30, 2024
6bf1227
Move all into logic
webjunkie Apr 30, 2024
437b291
Fix Cypress tests
webjunkie Apr 30, 2024
b6358f9
Fix jest test
webjunkie Apr 30, 2024
cd9657f
Fix test
webjunkie Apr 30, 2024
0a4510c
Reverse format
webjunkie Apr 30, 2024
12e940d
Update UI snapshots for `chromium` (1)
github-actions[bot] Apr 30, 2024
a46a0b4
Update UI snapshots for `chromium` (1)
github-actions[bot] Apr 30, 2024
20557e9
Fix spinner story
webjunkie Apr 30, 2024
a422c18
Update UI snapshots for `chromium` (1)
github-actions[bot] Apr 30, 2024
39dd51c
Update UI snapshots for `chromium` (1)
github-actions[bot] Apr 30, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions cypress/e2e/dashboard.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,11 @@ describe('Dashboard', () => {
cy.get('[data-attr=date-filter]').contains('No date range override')
cy.get('.InsightCard h5').should('have.length', 1).contains('Last 7 days')
// Override the time range on dashboard B
cy.get('button').contains('Edit filters').click()
cy.get('[data-attr=date-filter]').contains('No date range override').click()
cy.get('div').contains('Yesterday').should('exist').click()
cy.get('[data-attr=date-filter]').contains('Yesterday')
cy.get('button').contains('Apply and save dashboard').click()
cy.get('.InsightCard h5').should('have.length', 1).contains('Yesterday')
// Cool, now back to A and make sure the insight is still using the original range there, not the one from B
cy.clickNavMenu('dashboards')
Expand Down
6 changes: 6 additions & 0 deletions cypress/productAnalytics/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,21 +170,27 @@ export const dashboard = {
cy.wait('@postInsight')
},
addPropertyFilter(type: string = 'Browser', value: string = 'Chrome'): void {
cy.get('button').contains('Edit filters').click()
cy.get('.PropertyFilterButton').should('have.length', 0)
cy.get('[data-attr="property-filter-0"]').click()
cy.get('[data-attr="taxonomic-filter-searchfield"]').click().type('Browser').wait(1000)
cy.get('[data-attr="prop-filter-event_properties-0"]').click({ force: true })
cy.get('.LemonInput').type(value)
cy.contains('.LemonButton__content', value).click({ force: true })
cy.get('button').contains('Apply and save dashboard').click()
},
addAnyFilter(): void {
cy.get('button').contains('Edit filters').click()
cy.get('.PropertyFilterButton').should('have.length', 0)
cy.get('[data-attr="property-filter-0"]').click()
cy.get('[data-attr="taxonomic-filter-searchfield"]').click()
cy.get('[data-attr="prop-filter-event_properties-1"]').click({ force: true })
cy.get('[data-attr="prop-val"]').click()
cy.get('[data-attr="prop-val-0"]').click({ force: true })
// click .dashboard to blur
cy.get('.dashboard').click({ force: true })
cy.get('.PropertyFilterButton').should('have.length', 1)
cy.get('button').contains('Apply and save dashboard').click()
},
}

Expand Down
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,8 @@ export interface InsightCardProps extends Resizeable, React.HTMLAttributes<HTMLD
loadingQueued?: boolean
/** Whether the insight is loading. */
loading?: boolean
/** Whether the insight likely showing stale data. */
stale?: boolean
/** Whether an error occurred on the server. */
apiErrored?: boolean
/** Whether the card should be highlighted with a blue border. */
Expand Down Expand Up @@ -164,7 +166,7 @@ function VizComponentFallback(): JSX.Element {
}

export interface FilterBasedCardContentProps
extends Pick<InsightCardProps, 'insight' | 'loading' | 'apiErrored' | 'timedOut' | 'style'> {
extends Pick<InsightCardProps, 'insight' | 'loading' | 'apiErrored' | 'timedOut' | 'style' | 'stale'> {
insightProps: InsightLogicProps
tooFewFunnelSteps?: boolean
validationError?: string | null
Expand All @@ -185,6 +187,7 @@ export function FilterBasedCardContent({
tooFewFunnelSteps,
validationError,
context,
stale,
}: FilterBasedCardContentProps): JSX.Element {
const displayedType = getDisplayedType(insight.filters)
const VizComponent = displayMap[displayedType]?.element || VizComponentFallback
Expand All @@ -210,6 +213,7 @@ export function FilterBasedCardContent({
: undefined
}
>
{stale && !loading && <SpinnerOverlay mode="editing" />}
{loading && <SpinnerOverlay />}
{tooFewFunnelSteps ? (
<FunnelSingleStepState actionable={false} />
Expand All @@ -236,6 +240,7 @@ function InsightCardInternal(
ribbonColor,
loadingQueued,
loading,
stale,
apiErrored,
timedOut,
highlighted,
Expand Down Expand Up @@ -325,6 +330,7 @@ function InsightCardInternal(
insight={insight}
insightProps={insightLogicProps}
loading={loading}
stale={stale}
apiErrored={apiErrored}
timedOut={timedOut}
empty={empty}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ interface PropertyFiltersProps {
errorMessages?: JSX.Element[] | null
propertyAllowList?: { [key in TaxonomicFilterGroupType]?: string[] }
allowRelativeDateOptions?: boolean
disabled?: boolean
}

export function PropertyFilters({
Expand All @@ -63,6 +64,7 @@ export function PropertyFilters({
errorMessages = null,
propertyAllowList,
allowRelativeDateOptions,
disabled = false,
}: PropertyFiltersProps): JSX.Element {
const logicProps = { propertyFilters, onChange, pageKey, sendAllKeyUpdates }
const { filters, filtersWithNew } = useValues(propertyFilterLogic(logicProps))
Expand Down Expand Up @@ -128,6 +130,7 @@ export function PropertyFilters({
)}
errorMessage={errorMessages && errorMessages[index]}
openOnInsert={allowOpenOnInsert && openOnInsert}
disabled={disabled}
/>
</React.Fragment>
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ interface FilterRowProps {
onRemove: (index: number) => void
orFiltering?: boolean
errorMessage?: JSX.Element | null
disabled?: boolean
}

export const FilterRow = React.memo(function FilterRow({
Expand All @@ -42,6 +43,7 @@ export const FilterRow = React.memo(function FilterRow({
onRemove,
orFiltering,
errorMessage,
disabled,
}: FilterRowProps) {
const [open, setOpen] = useState(false)

Expand Down Expand Up @@ -92,8 +94,9 @@ export const FilterRow = React.memo(function FilterRow({
onClick={() => setOpen(!open)}
onClose={() => onRemove(index)}
item={item}
disabled={disabled}
/>
) : (
) : !disabled ? (
<LemonButton
onClick={() => setOpen(!open)}
className="new-prop-filter"
Expand All @@ -105,7 +108,7 @@ export const FilterRow = React.memo(function FilterRow({
>
{label}
</LemonButton>
)}
) : undefined}
</Popover>
)}
{key && showConditionBadge && index + 1 < totalCount && <OperandTag operand="and" />}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,12 @@
outline: 0;
transition: all 0.2s cubic-bezier(0.645, 0.045, 0.355, 1);

&:hover,
&[aria-disabled='true']:not(.LemonButton--loading) {
cursor: not-allowed;
opacity: var(--opacity-disabled);
}

&:hover:not([aria-disabled='true']),
&:focus {
border-color: var(--border-bold);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,11 @@ export interface PropertyFilterButtonProps {
onClose?: () => void
children?: string
item: AnyPropertyFilter
disabled?: boolean
}

export const PropertyFilterButton = React.forwardRef<HTMLElement, PropertyFilterButtonProps>(
function PropertyFilterButton({ onClick, onClose, children, item }, ref): JSX.Element {
function PropertyFilterButton({ onClick, onClose, children, item, disabled }, ref): JSX.Element {
const { cohortsById } = useValues(cohortsModel)
const { formatPropertyValueForDisplay } = useValues(propertyDefinitionsModel)

Expand All @@ -37,18 +38,19 @@ export const PropertyFilterButton = React.forwardRef<HTMLElement, PropertyFilter
return (
<ButtonComponent
ref={ref as any}
onClick={onClick}
onClick={disabled ? undefined : onClick}
className={clsx('PropertyFilterButton', {
'PropertyFilterButton--closeable': closable,
'PropertyFilterButton--clickable': clickable,
'ph-no-capture': true,
})}
aria-disabled={disabled}
>
<PropertyFilterIcon type={item.type} />
<span className="PropertyFilterButton-content" title={label}>
{midEllipsis(label, 32)}
</span>
{closable && (
{closable && !disabled && (
<LemonButton
size="xsmall"
icon={<IconX />}
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/lib/lemon-ui/Spinner/Spinner.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ export function AsOverlay(): JSX.Element {
)
}

export function asOverlayWaiting(): JSX.Element {
export function asOverlayEditing(): JSX.Element {
return (
<div className="relative">
<h1>Hey there</h1>
Expand All @@ -109,7 +109,7 @@ export function asOverlayWaiting(): JSX.Element {
about to happen.
</p>

<SpinnerOverlay mode="waiting" />
<SpinnerOverlay mode="editing" />
</div>
)
}
8 changes: 4 additions & 4 deletions frontend/src/lib/lemon-ui/Spinner/Spinner.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import './Spinner.scss'

import { IconClock } from '@posthog/icons'
import { IconPencil } from '@posthog/icons'
import { twJoin, twMerge } from 'tailwind-merge'

export interface SpinnerProps {
Expand Down Expand Up @@ -38,12 +38,12 @@ export function SpinnerOverlay({
/** @default true */
visible?: boolean
/** @default "spinning" */
mode?: 'spinning' | 'waiting'
mode?: 'spinning' | 'editing'
}): JSX.Element {
return (
<div className={twJoin('SpinnerOverlay', sceneLevel && 'SpinnerOverlay--scene-level')} aria-hidden={!visible}>
{mode === 'waiting' ? (
<IconClock className="text-5xl text-primary z-10 animate-pulse drop-shadow-xl" />
{mode === 'editing' ? (
<IconPencil className="text-5xl text-primary z-10 drop-shadow-xl" />
) : (
<Spinner className={twMerge('text-5xl', className)} {...spinnerProps} />
)}
Expand Down
59 changes: 10 additions & 49 deletions frontend/src/scenes/dashboard/Dashboard.tsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,10 @@
import { IconCalendar } from '@posthog/icons'
import { LemonButton } from '@posthog/lemon-ui'
import { BindLogic, useActions, useValues } from 'kea'
import { DateFilter } from 'lib/components/DateFilter/DateFilter'
import { NotFound } from 'lib/components/NotFound'
import { PropertyFilters } from 'lib/components/PropertyFilters/PropertyFilters'
import { TaxonomicFilterGroupType } from 'lib/components/TaxonomicFilter/types'
import { useKeyboardHotkeys } from 'lib/hooks/useKeyboardHotkeys'
import { DashboardEventSource } from 'lib/utils/eventUsageLogic'
import { useEffect } from 'react'
import { DashboardEditBar } from 'scenes/dashboard/DashboardEditBar'
import { DashboardItems } from 'scenes/dashboard/DashboardItems'
import { dashboardLogic, DashboardLogicProps } from 'scenes/dashboard/dashboardLogic'
import { DashboardReloadAction, LastRefreshText } from 'scenes/dashboard/DashboardReloadAction'
Expand All @@ -17,7 +14,6 @@ import { urls } from 'scenes/urls'

import { DashboardMode, DashboardPlacement, DashboardType } from '~/types'

import { groupsModel } from '../../models/groupsModel'
import { DashboardHeader } from './DashboardHeader'
import { EmptyDashboardComponent } from './EmptyDashboardComponent'

Expand Down Expand Up @@ -45,19 +41,9 @@ export function Dashboard({ id, dashboard, placement }: DashboardProps = {}): JS
}

function DashboardScene(): JSX.Element {
const {
placement,
dashboard,
canEditDashboard,
tiles,
itemsLoading,
filters: dashboardFilters,
dashboardMode,
dashboardFailedToLoad,
} = useValues(dashboardLogic)
const { setDashboardMode, setDates, reportDashboardViewed, setProperties, abortAnyRunningQuery } =
useActions(dashboardLogic)
const { groupsTaxonomicTypes } = useValues(groupsModel)
const { placement, dashboard, canEditDashboard, tiles, itemsLoading, dashboardMode, dashboardFailedToLoad } =
useValues(dashboardLogic)
const { setDashboardMode, reportDashboardViewed, abortAnyRunningQuery } = useActions(dashboardLogic)

useEffect(() => {
reportDashboardViewed()
Expand Down Expand Up @@ -115,37 +101,12 @@ function DashboardScene(): JSX.Element {
DashboardPlacement.Public,
DashboardPlacement.Export,
DashboardPlacement.FeatureFlag,
].includes(placement) && (
<div className="flex space-x-4 items-center">
<DateFilter
showCustom
dateFrom={dashboardFilters?.date_from ?? undefined}
dateTo={dashboardFilters?.date_to ?? undefined}
onChange={setDates}
disabled={!canEditDashboard}
makeLabel={(key) => (
<>
<IconCalendar />
<span className="hide-when-small"> {key}</span>
</>
)}
/>
<PropertyFilters
onChange={setProperties}
pageKey={'dashboard_' + dashboard?.id}
propertyFilters={dashboard?.filters.properties}
taxonomicGroupTypes={[
TaxonomicFilterGroupType.EventProperties,
TaxonomicFilterGroupType.PersonProperties,
TaxonomicFilterGroupType.EventFeatureFlags,
...groupsTaxonomicTypes,
TaxonomicFilterGroupType.Cohorts,
TaxonomicFilterGroupType.Elements,
TaxonomicFilterGroupType.HogQLExpression,
]}
/>
</div>
)}
].includes(placement) &&
dashboard && (
<div className="flex space-x-4 items-center">
<DashboardEditBar />
</div>
)}
{placement === DashboardPlacement.FeatureFlag && dashboard?.id && (
<LemonButton type="secondary" size="small" to={urls.dashboard(dashboard.id)}>
Edit dashboard
Expand Down
67 changes: 67 additions & 0 deletions frontend/src/scenes/dashboard/DashboardEditBar.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import { IconCalendar } from '@posthog/icons'
import { LemonButton } from '@posthog/lemon-ui'
import { useActions, useValues } from 'kea'
import { DateFilter } from 'lib/components/DateFilter/DateFilter'
import { PropertyFilters } from 'lib/components/PropertyFilters/PropertyFilters'
import { TaxonomicFilterGroupType } from 'lib/components/TaxonomicFilter/types'
import { dashboardLogic } from 'scenes/dashboard/dashboardLogic'

import { groupsModel } from '~/models/groupsModel'

export function DashboardEditBar(): JSX.Element {
const { dashboard, canEditDashboard, editMode, temporaryFilters, stale } = useValues(dashboardLogic)
const { setDates, setProperties, cancelTemporary, applyTemporary, setEditMode } = useActions(dashboardLogic)
const { groupsTaxonomicTypes } = useValues(groupsModel)

return (
<div className="flex gap-2 items-center justify-between flex-wrap">
<DateFilter
showCustom
dateFrom={temporaryFilters.date_from}
dateTo={temporaryFilters.date_to}
onChange={setDates}
disabled={!canEditDashboard || !editMode}
makeLabel={(key) => (
<>
<IconCalendar />
<span className="hide-when-small"> {key}</span>
</>
)}
/>
<PropertyFilters
disabled={!canEditDashboard || !editMode}
onChange={setProperties}
pageKey={'dashboard_' + dashboard?.id}
propertyFilters={temporaryFilters.properties}
taxonomicGroupTypes={[
TaxonomicFilterGroupType.EventProperties,
TaxonomicFilterGroupType.PersonProperties,
TaxonomicFilterGroupType.EventFeatureFlags,
...groupsTaxonomicTypes,
TaxonomicFilterGroupType.Cohorts,
TaxonomicFilterGroupType.Elements,
TaxonomicFilterGroupType.HogQLExpression,
]}
/>
{canEditDashboard && !editMode ? (
<LemonButton type="secondary" size="small" onClick={() => setEditMode(true)}>
Edit filters
</LemonButton>
) : (
<>
<LemonButton onClick={cancelTemporary} type="secondary" size="small" className="ml-4">
Cancel
</LemonButton>
<LemonButton
onClick={applyTemporary}
type="primary"
size="small"
disabledReason={!stale ? 'No changes to apply' : undefined}
>
Apply and save dashboard
</LemonButton>
</>
)}
</div>
)
}