feat: Add initial Core Web Vitals UI behind FF #27336

rafaeelaudibert · 2025-01-07T21:28:31Z

First PR on the Core Web Vitals project!

Changes

This is a simple PR creating the page and route for our upcoming Core Web Vitals UI
There're some slight changes to some components so that our <Filters /> component has the proper space to render on Mobile

This is hidden behind a FF for now because there isn't anything to see, but it should go behind a Beta FF soon!

https://www.loom.com/share/2ac44609fe894f9182ea76a7e0d7a20b?sid=cab54855-c9b6-40f8-bba8-579c676f89d3

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

Yes sir

How did you test this code?

Manual testing, looked around the app comparing with prod to guarantee changes to Filters had no regressions around the app

rafaeelaudibert · 2025-01-07T21:54:54Z

frontend/src/lib/hooks/useWindowSize.js

Moved this to a .ts file but git wasn't smart enough to notice that, see below

rafaeelaudibert · 2025-01-07T21:55:45Z

frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx

@@ -688,8 +708,31 @@ export const webAnalyticsLogic = kea<webAnalyticsLogicType>([
                    }
                }

-                const pathCleaningSettingsUrl = urls.settings('project-product-analytics', 'path-cleaning')
-                const customChannelTypesUrl = urls.settings('environment-web-analytics', 'channel-type')
+                // TODO: Use actual web vitals tab, this is just a placeholder


Not happening on this PR, this will be a thing on the next PR

rafaeelaudibert · 2025-01-07T21:56:19Z

frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx

+        '/web': (pathParams, queryParams) => setFilters(actions, values, pathParams, queryParams),
+        '/web/:productTab': (pathParams, queryParams) => setFilters(actions, values, pathParams, queryParams),


I couldn't get this to work with a single string. I tried all sorts of regexes, but I don't think kea is that powerful. @mariusandra if you're keen to help here :)

I'm not sure what you're trying to achieve, and Kea is as powerful as the user 😅. urlToAction takes a function that returns an object, so feel free to add whatever rows into the object that you need to.

urlToAction(({ actions, values }) => { const obj = {} const handler = (path, search, hash) => {...} for (...) { obj[`/web/${product.name}`] => handler } return obj })

Oh, of course, it's a function, I can just create the keys programatically, thanks

rafaeelaudibert · 2025-01-07T21:57:13Z

frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx

+    if (device_tab && device_tab !== values._deviceTab) {
+        actions.setDeviceTab(device_tab)
+    }


I would love so much if our style allowed this to be inlined, it's just so clean. One can only wonder ⭐

Suggested change

if (device_tab && device_tab !== values._deviceTab) {

actions.setDeviceTab(device_tab)

}

if (device_tab && device_tab !== values._deviceTab) actions.setDeviceTab(device_tab)

And there are others who'll consider the straightforwardness of the non-inline version as clean and elegant.

We don't support inline ifs to avoid bugs, which happen much easier this ways. While you or I might not make any, considering how this company is growing and how many people work on the codebase, we need some structure to maintain sanity :).

We don't support inline ifs to avoid bugs, which happen much easier this ways.

Yeah, I can get behind, hence why I won't propose changing this, I can withstand spending 2 seconds adding curly braces if we can avoid the odd bug.

rafaeelaudibert · 2025-01-07T21:57:47Z

frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx

    }),
    selectors(({ actions, values }) => ({
+        breadcrumbs: [


I'm still not sure how breadcrumbs work. Who's accessing these? This is like magic to me

Whatever is your active scene, and what is that scene's logic and component, is recorded in sceneLogic (each scene exports its data with export const scene: SceneExport = ...). If the scene's logic contains a breadcrumbs selector, we use it to show breadcrumbs via breadcrumbsLogic

If the scene's logic contains a breadcrumbs selector

Ah ok, that's the confusing bit for me. So we're dynamically accessing the logic exported on SceneExport and then conditionally checking if they have a breadcrumbs selector? Sounds like typescript hell, but that makes sense. And then, if there's no selector, we simply use the SceneExport title as the crumb, right?

posthog-bot · 2025-01-07T22:14:09Z

📸 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, diff for shard 2)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

github-actions · 2025-01-07T22:28:19Z

Size Change: +87 B (+0.01%)

Total Size: 1.11 MB

ℹ️ View Unchanged

Filename	Size	Change
`frontend/dist/toolbar.js`	1.11 MB	+87 B (+0.01%)

_{compressed-size-action}

posthog-bot · 2025-01-07T22:34:34Z

📸 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 · 2025-01-08T08:12:07Z

frontend/src/lib/hooks/useWindowSize.ts

You could possibly also use this: https://keajs.org/docs/plugins/window-values/

Yes, but then I need to import TAILWIND_BREAKPOINTS into every logic that wants to know the screen size and reimplement the exact same logic. Using this hook should be much simpler.

Reimplement the exact same logic? Not unless you make one common tailwindBreakpointLogic and use that :D

mariusandra · 2025-01-08T08:15:19Z

frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx

    }),
    selectors(({ actions, values }) => ({
+        breadcrumbs: [


Whatever is your active scene, and what is that scene's logic and component, is recorded in sceneLogic (each scene exports its data with export const scene: SceneExport = ...). If the scene's logic contains a breadcrumbs selector, we use it to show breadcrumbs via breadcrumbsLogic

mariusandra · 2025-01-08T08:16:23Z

frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx

+        '/web': (pathParams, queryParams) => setFilters(actions, values, pathParams, queryParams),
+        '/web/:productTab': (pathParams, queryParams) => setFilters(actions, values, pathParams, queryParams),


I'm not sure what you're trying to achieve, and Kea is as powerful as the user 😅. urlToAction takes a function that returns an object, so feel free to add whatever rows into the object that you need to.

urlToAction(({ actions, values }) => { const obj = {} const handler = (path, search, hash) => {...} for (...) { obj[`/web/${product.name}`] => handler } return obj })

robbie-c

Sure, LGTM.

Long term we might want to separate these into difference scenes/logics, I don't know how much code we'll be sharing between the two views but even if it's a lot, that might be easier than having one scene for both.

Keen to see the product itself :)

rafaeelaudibert · 2025-01-08T12:41:39Z

Long term we might want to separate these into difference scenes/logics, I don't know how much code we'll be sharing between the two views but even if it's a lot, that might be easier than having one scene for both.

@robbie-c Yeah, fully agreed. I reused it because I thought it was easier right now - while I don't understand kea fully. I thought we'd share a lot of code, but we'll probably only share the header, but that's easy to solve, I can just extract that to a separate component.

robbie-c · 2025-01-08T12:43:58Z

I don't think you need to worry about this too much right now though, let's focus on getting something in front of our users, even if it means creating some tech debt which we can clean up later

This will allow us to more easily check whether we're on a small screen or not. Without this, we'd need to keep implementing our own breakpoint checking by using `TAILWIND_BREAKPOINTS`

This is a simple commit creating the page and route for our upcoming Core Web Vitals UI There's some slight changes to some components so that our `<Filters />` component has the proper space to render

We changed how our filters work in the previous commit because we wanted a nicer layout for Web Analytics, but it added a couple of regressions in some other parts of the app, let's fix them :)

Even though these aren't used in the app, we need them to be exported if we want the typegen to work

This more closely explains what's this function purpose, and it also allows us to heavily simplify our types

rafaeelaudibert requested a review from robbie-c January 7, 2025 21:28

rafaeelaudibert force-pushed the add-web-vitals-secondary-page-to-web-analytics branch from a65cd6b to b32deb8 Compare January 7, 2025 21:29

rafaeelaudibert changed the title ~~Add initial Core Web Vitals UI behind FF~~ feat: Add initial Core Web Vitals UI behind FF Jan 7, 2025

rafaeelaudibert commented Jan 7, 2025

View reviewed changes

mariusandra reviewed Jan 8, 2025

View reviewed changes

robbie-c approved these changes Jan 8, 2025

View reviewed changes

rafaeelaudibert and others added 12 commits January 8, 2025 11:51

feat: Add new isWindowLessThan function to useWindowSize

687b08f

This will allow us to more easily check whether we're on a small screen or not. Without this, we'd need to keep implementing our own breakpoint checking by using `TAILWIND_BREAKPOINTS`

feat: Add initial Core Web Vitals UI behind FF

c86eea0

This is a simple commit creating the page and route for our upcoming Core Web Vitals UI There's some slight changes to some components so that our `<Filters />` component has the proper space to render

layout: Small fixes after changes to PropertyFilters

182b2b0

We changed how our filters work in the previous commit because we wanted a nicer layout for Web Analytics, but it added a couple of regressions in some other parts of the app, let's fix them :)

bug: Properly change product tab when clicking on the sidebar

df6ecbf

bug: Add missing scene for Core Web Vitals

1a13946

bug: Fix missing export

7007e18

Even though these aren't used in the app, we need them to be exported if we want the typegen to work

feat: Add Core Web Vitals/Web Analytics breadcrumbs

9ea7518

feat: Add docs path to web analytics scene

e3f87e3

Update UI snapshots for chromium (1)

b52c471

Update UI snapshots for chromium (2)

61cadb3

Update UI snapshots for chromium (2)

27caacf

refactor: Hide setFilters function inside urlToAction

c0910c3

This more closely explains what's this function purpose, and it also allows us to heavily simplify our types

rafaeelaudibert force-pushed the add-web-vitals-secondary-page-to-web-analytics branch from 901bf09 to c0910c3 Compare January 8, 2025 14:51

rafaeelaudibert enabled auto-merge (squash) January 8, 2025 14:51

rafaeelaudibert merged commit adc845f into master Jan 8, 2025
99 checks passed

rafaeelaudibert deleted the add-web-vitals-secondary-page-to-web-analytics branch January 8, 2025 17:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add initial Core Web Vitals UI behind FF #27336

feat: Add initial Core Web Vitals UI behind FF #27336

rafaeelaudibert commented Jan 7, 2025 •

edited

Loading

rafaeelaudibert Jan 7, 2025

rafaeelaudibert Jan 7, 2025

rafaeelaudibert Jan 7, 2025

mariusandra Jan 8, 2025

rafaeelaudibert Jan 8, 2025

rafaeelaudibert Jan 7, 2025

mariusandra Jan 8, 2025

rafaeelaudibert Jan 8, 2025 •

edited

Loading

rafaeelaudibert Jan 7, 2025

mariusandra Jan 8, 2025

rafaeelaudibert Jan 8, 2025

posthog-bot commented Jan 7, 2025

github-actions bot commented Jan 7, 2025 •

edited

Loading

posthog-bot commented Jan 7, 2025

mariusandra Jan 8, 2025

rafaeelaudibert Jan 8, 2025

mariusandra Jan 10, 2025

mariusandra Jan 8, 2025

mariusandra Jan 8, 2025

robbie-c left a comment

rafaeelaudibert commented Jan 8, 2025

robbie-c commented Jan 8, 2025

		'/web': (pathParams, queryParams) => setFilters(actions, values, pathParams, queryParams),
		'/web/:productTab': (pathParams, queryParams) => setFilters(actions, values, pathParams, queryParams),

feat: Add initial Core Web Vitals UI behind FF #27336

feat: Add initial Core Web Vitals UI behind FF #27336

Conversation

rafaeelaudibert commented Jan 7, 2025 • edited Loading

Changes

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

How did you test this code?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rafaeelaudibert Jan 8, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

posthog-bot commented Jan 7, 2025

📸 UI snapshots have been updated

github-actions bot commented Jan 7, 2025 • edited Loading

posthog-bot commented Jan 7, 2025

📸 UI snapshots have been updated

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robbie-c left a comment

Choose a reason for hiding this comment

rafaeelaudibert commented Jan 8, 2025

robbie-c commented Jan 8, 2025

rafaeelaudibert commented Jan 7, 2025 •

edited

Loading

rafaeelaudibert Jan 8, 2025 •

edited

Loading

github-actions bot commented Jan 7, 2025 •

edited

Loading