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

webjunkie · 2024-04-25T08:59:26Z

Problem

I read about some confusion that adjusting the filters saves the insight for everyone in the team immediately.

Changes

disable filters and put an edit button

Does this work well for both Cloud and self-hosted?

n/a

How did you test this code?

tbd

posthog-bot · 2024-04-25T09:09:32Z

📸 UI snapshots have been updated

4 snapshot changes in total. 0 added, 4 modified, 0 deleted:

chromium: 0 added, 4 modified, 0 deleted (diff for shard 1)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

github-actions · 2024-04-25T09:17:46Z

Size Change: +524 B (0%)

Total Size: 1.05 MB

ℹ️ View Unchanged

Filename	Size	Change
`frontend/dist/toolbar.js`	1.05 MB	+524 B (0%)

_{compressed-size-action}

posthog-bot · 2024-04-25T09:19:45Z

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot · 2024-04-25T09:29:57Z

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

webjunkie · 2024-04-25T09:56:10Z

@mariusandra and @Twixes is this something that would make sense?

mariusandra · 2024-04-25T10:59:09Z

In general, improving this makes sense, but I'd change the flow here:

I expected that when I change the filters, the dashboard would update automatically, just not save each insight
Then I could press "save" to persist the changes, or "cancel" (or "reset") to reset them.

Is it easy to add a "soft apply" on filter change?

Also, do we want to show if any filters are applied? A visually easy way to do it would be a red notification bubble/dot with the number of filters inside.

webjunkie · 2024-04-25T11:04:11Z

Thanks for the input!

the dashboard would update automatically, just not save each insight

Yeah this soft apply or preview is exactly the problem.. we don't have it. It saves the whole dashboard and only this applies the filters to all insights and then you can recalculate them to see them.
So even "hey let's update and then roll back" would mean that meanwhile someone else opening the dashboard somewhere else would see your changes.

Not sure what to make of it. Maybe we make all insights grey when you adjust the filters until you save or cancel? 🙈

A visually easy way to do it would be a red notification bubble

Good idea. And that also leads me to the realization that now with my change you don't see what this dashboard's filter and time is.

webjunkie · 2024-04-25T12:20:57Z

New idea:

Show filters as disabled. Show insights as stale while editing filters.

posthog-bot · 2024-04-25T12:29:40Z

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

chromium: 0 added, 1 modified, 0 deleted (diff for shard 1)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot · 2024-04-29T09:57:45Z

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot · 2024-04-29T10:07:59Z

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

mariusandra

I think this new option is good enough to try UX-wise. Code wise, I think the "stale" insight card state is no longer needed (adding dead code 🤔), and I also think a bit of state management could move from React to the Kea layer... possibly?

mariusandra · 2024-04-29T15:05:13Z

frontend/src/scenes/dashboard/DashboardEditBar.tsx

+    const [editMode, setEditMode] = useState(false)
+    const [tempDates, setTempDates] = useState<Dates>({
+        dateFrom: dashboardFilters?.date_from,
+        dateTo: dashboardFilters?.date_to,
+    })
+    const [tempProperties, setTempProperties] = useState<AnyPropertyFilter[] | undefined>(
+        dashboard?.filters.properties ?? undefined
+    )
+
+    useEffect(() => {
+        const hasPendingChanges =
+            tempDates.dateFrom !== dashboardFilters?.date_from ||
+            tempDates.dateTo !== dashboardFilters?.date_to ||
+            JSON.stringify(tempProperties) !== JSON.stringify(dashboard?.filters.properties)
+        if (onPendingChanges) {
+            onPendingChanges(hasPendingChanges)
+        }
+    }, [tempDates, tempProperties])
+
+    const handleSave: () => void = () => {
+        if (!canEditDashboard) {
+            return
+        }
+        setDates(tempDates.dateFrom ?? null, tempDates.dateTo ?? null)
+        if (tempProperties) {
+            setProperties(tempProperties)
+        }
+        if (onPendingChanges) {
+            onPendingChanges(false)
+        }
+        setEditMode(false)
+    }
+
+    const handleCancel: () => void = () => {
+        setTempDates({
+            dateFrom: dashboardFilters?.date_from,
+            dateTo: dashboardFilters?.date_to,
+        })
+        setTempProperties(dashboard?.filters.properties ?? undefined)
+        setEditMode(false)
+        if (onPendingChanges) {
+            onPendingChanges(false)
+        }
+    }


This looks like it could be a logic. We do try to avoid react state as much as possible

webjunkie · 2024-04-30T08:37:02Z

I think the "stale" insight card state is no longer needed (adding dead code 🤔)

Not sure I follow. I made it undead, added the stale. It shows up when you change filters so you don't wonder why nothing is happening. Do you mean this?

# Conflicts: # frontend/__snapshots__/posthog-3000-navigation--navigation-3000--dark.png # frontend/__snapshots__/posthog-3000-navigation--navigation-3000--light.png # frontend/__snapshots__/posthog-3000-navigation--navigation-base--dark.png # frontend/__snapshots__/posthog-3000-navigation--navigation-base--light.png

mariusandra · 2024-04-30T09:33:04Z

I think the "stale" insight card state is no longer needed (adding dead code 🤔)

Not sure I follow. I made it undead, added the stale. It shows up when you change filters so you don't wonder why nothing is happening. Do you mean this?

Ignore me then 😬

posthog-bot · 2024-04-30T11:35:04Z

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

chromium: 0 added, 2 modified, 0 deleted (diff for shard 1)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot · 2024-04-30T11:44:25Z

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

chromium: 0 added, 2 modified, 0 deleted (diff for shard 1)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot · 2024-04-30T12:04:01Z

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

chromium: 0 added, 2 modified, 0 deleted (diff for shard 1)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot · 2024-04-30T12:13:24Z

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

chromium: 0 added, 2 modified, 0 deleted (diff for shard 1)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

webjunkie requested a review from a team April 26, 2024 07:37

Twixes force-pushed the fix/dashboard-logic-insights branch from 31526a6 to 16e9855 Compare April 26, 2024 13:06

Base automatically changed from fix/dashboard-logic-insights to master April 29, 2024 06:25

webjunkie and others added 9 commits April 29, 2024 11:46

Add edit switch to dashboard to make clear these are changing dashboard

39a8d5c

Add DashboardEditBar to encapsulate

5e0b618

Update UI snapshots for chromium (1)

f5f9949

Update UI snapshots for chromium (2)

9d0aff7

Update UI snapshots for chromium (2)

e4bd5b2

Disable and do not hide filters

495d876

Mark dashboard stale

39e96dc

Update UI snapshots for chromium (1)

f5d4877

Fix taxonomicGroupTypes prop

a743bd5

webjunkie force-pushed the feature/dashboard-edit-switch branch from 40be2aa to a743bd5 Compare April 29, 2024 09:47

webjunkie requested a review from mariusandra April 29, 2024 09:47

Update UI snapshots for chromium (2)

3f6c277

Update UI snapshots for chromium (2)

c8b172f

mariusandra reviewed Apr 29, 2024

View reviewed changes

Move logic inside DashboardEditBar

9629517

webjunkie added 5 commits April 30, 2024 12:04

Move all into logic

6bf1227

Fix Cypress tests

437b291

Fix jest test

b6358f9

Fix test

cd9657f

Reverse format

0a4510c

webjunkie enabled auto-merge (squash) April 30, 2024 11:26

Update UI snapshots for chromium (1)

12e940d

Update UI snapshots for chromium (1)

a46a0b4

webjunkie and others added 2 commits April 30, 2024 13:49

Fix spinner story

20557e9

Update UI snapshots for chromium (1)

a422c18

Update UI snapshots for chromium (1)

39dd51c

webjunkie merged commit a63217b into master Apr 30, 2024
103 checks passed

webjunkie deleted the feature/dashboard-edit-switch branch April 30, 2024 12:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

webjunkie commented Apr 25, 2024 •

edited

posthog-bot commented Apr 25, 2024

github-actions bot commented Apr 25, 2024 •

edited

posthog-bot commented Apr 25, 2024

posthog-bot commented Apr 25, 2024

webjunkie commented Apr 25, 2024

mariusandra commented Apr 25, 2024

webjunkie commented Apr 25, 2024

webjunkie commented Apr 25, 2024 •

edited

posthog-bot commented Apr 25, 2024

posthog-bot commented Apr 29, 2024

posthog-bot commented Apr 29, 2024

mariusandra left a comment

mariusandra Apr 29, 2024

webjunkie commented Apr 30, 2024

mariusandra commented Apr 30, 2024

posthog-bot commented Apr 30, 2024

posthog-bot commented Apr 30, 2024

posthog-bot commented Apr 30, 2024

posthog-bot commented Apr 30, 2024

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

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

Conversation

webjunkie commented Apr 25, 2024 • edited

Problem

Changes

Does this work well for both Cloud and self-hosted?

How did you test this code?

posthog-bot commented Apr 25, 2024

📸 UI snapshots have been updated

github-actions bot commented Apr 25, 2024 • edited

posthog-bot commented Apr 25, 2024

📸 UI snapshots have been updated

posthog-bot commented Apr 25, 2024

📸 UI snapshots have been updated

webjunkie commented Apr 25, 2024

mariusandra commented Apr 25, 2024

webjunkie commented Apr 25, 2024

webjunkie commented Apr 25, 2024 • edited

posthog-bot commented Apr 25, 2024

📸 UI snapshots have been updated

posthog-bot commented Apr 29, 2024

📸 UI snapshots have been updated

posthog-bot commented Apr 29, 2024

📸 UI snapshots have been updated

mariusandra left a comment

Choose a reason for hiding this comment

mariusandra Apr 29, 2024

Choose a reason for hiding this comment

webjunkie commented Apr 30, 2024

mariusandra commented Apr 30, 2024

posthog-bot commented Apr 30, 2024

📸 UI snapshots have been updated

posthog-bot commented Apr 30, 2024

📸 UI snapshots have been updated

posthog-bot commented Apr 30, 2024

📸 UI snapshots have been updated

posthog-bot commented Apr 30, 2024

📸 UI snapshots have been updated

webjunkie commented Apr 25, 2024 •

edited

github-actions bot commented Apr 25, 2024 •

edited

webjunkie commented Apr 25, 2024 •

edited