feat(navigation): drop starred sidebar item, instrument files tree - #73518
Conversation
|
Hey @fercgomes! 👋 It looks like your git author email on this PR isn't your
You can fix it for this repo with: git config user.email "you@posthog.com"Or set it globally with |
🦔 Hogbox preview · ✅ ready▶ Open the preview
commit |
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
frontend/src/layout/panel-layout/ProjectTree/projectTreeLogic.tsx:1223
**Default project root becomes null**
When a Files tree instance omits `root`, the logic treats it as the project root but these captures serialize it as `null`, causing interactions from those instances to be excluded when analytics filters for `root: "project://"`.
### Issue 2 of 2
frontend/src/layout/panel-layout/ProjectTree/projectTreeLogic.tsx:1368-1373
**Bulk move count includes skipped items**
When a selected folder has selected descendants, `checkedItemCountNumeric` counts every checked entry while the following `skipInFolder` loop deliberately performs only the parent move, causing the event's `count` to overstate the number of items moved.
Reviews (1): Last reviewed commit: "feat(navigation): drop starred sidebar i..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (searchResults.searchTerm && (!payload || !payload.offset)) { | ||
| posthog.capture('project tree searched', { | ||
| root: props.root ?? null, | ||
| has_results: searchResults.results.length > 0, |
There was a problem hiding this comment.
When a Files tree instance omits root, the logic treats it as the project root but these captures serialize it as null, causing interactions from those instances to be excluded when analytics filters for root: "project://".
Knowledge Base Used: Frontend app (frontend/src)
Prompt To Fix With AI
This is a comment left during a code review.
Path: frontend/src/layout/panel-layout/ProjectTree/projectTreeLogic.tsx
Line: 1223
Comment:
**Default project root becomes null**
When a Files tree instance omits `root`, the logic treats it as the project root but these captures serialize it as `null`, causing interactions from those instances to be excluded when analytics filters for `root: "project://"`.
**Knowledge Base Used:** [Frontend app (`frontend/src`)](https://app.greptile.com/posthog-org-19734/-/custom-context/knowledge-base/posthog/posthog/-/docs/frontend-app.md)
How can I resolve this? If you propose a fix, please make it concise.
🤖 CI report
|
| Root | Eager (shipped) | Δ vs base | Budget |
|---|---|---|---|
entry (logged-out pages, app bootstrap)src/index.tsx |
1.24 MiB · 22 files | no change | ███░░░░░░░ 27.6% of 4.51 MiB |
authenticated shell (every logged-in page)src/scenes/AuthenticatedShell.tsx |
8.07 MiB · 3,011 files | 🔺 +773 B (+0.0%) | ████████░░ 83.1% of 9.71 MiB |
🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 node_modules/monaco-editor/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
Largest files eagerly shipped from src/index.tsx
| Size | File |
|---|---|
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 24.6 KiB | ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js |
| 6.3 KiB | ../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js |
| 4.5 KiB | ../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js |
| 3.9 KiB | ../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js |
| 1.4 KiB | ../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js |
| 1.3 KiB | src/RootErrorBoundary.tsx |
| 912 B | ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js |
| 789 B | src/scenes/ChunkLoadErrorBoundary.tsx |
| 762 B | src/index.tsx |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
| Size | File |
|---|---|
| 281.5 KiB | ../node_modules/.pnpm/posthog-js@1.407.2/node_modules/posthog-js/dist/rrweb.js |
| 267.7 KiB | ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 236.0 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 226.1 KiB | ../node_modules/.pnpm/posthog-js@1.407.2/node_modules/posthog-js/dist/module.js |
| 154.3 KiB | ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 106.2 KiB | src/lib/api.ts |
| 94.0 KiB | ../packages/quill/packages/quill/dist/index.js |
| 93.3 KiB | ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js |
| 90.6 KiB | ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js |
Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479
✅ Toolbar bundle — eager 2.18 MiB within budget
What the toolbar ships to customer pages, measured from the esbuild output (minified, post-tree-shake). The eager set is the entry plus everything statically imported from it — fetched before any feature runs; deferred chunks load lazily. The eager guardrail is 5.72 MiB. Each output file must also stay below 10 MB, where CloudFront stops compressing it. The module boundary is enforced separately by check-toolbar-graph.
| Metric | Size | Δ vs base | Budget |
|---|---|---|---|
| Eager (shipped) entry + static imports |
2.18 MiB · 17 files | no change | ████░░░░░░ 38.1% of 5.72 MiB |
| Deferred (lazy) | 2.07 MiB · 33 files | no change | n/a — loads on demand |
Loader dist/toolbar.js |
1.1 KiB | no change | █░░░░░░░░░ 5.8% of 19.5 KiB |
Largest eagerly-shipped chunks
| Size | File |
|---|---|
| 716.0 KiB | dist/toolbar/toolbar-app-UT2TAV4U.css |
| 545.1 KiB | dist/toolbar/chunk-chunk-R67YSTOI.js |
| 484.3 KiB | dist/toolbar/chunk-chunk-PY4X2WR2.js |
| 133.6 KiB | dist/toolbar/chunk-chunk-P36RTWFE.js |
| 131.8 KiB | dist/toolbar/chunk-chunk-T5KY5WYR.js |
| 71.0 KiB | dist/toolbar/toolbar-app-KIASA47E.js |
| 69.0 KiB | dist/toolbar/chunk-chunk-27JL52RE.js |
| 35.6 KiB | dist/toolbar/chunk-chunk-NZ4IQ3HA.js |
| 20.9 KiB | dist/toolbar/chunk-chunk-DKDYNNXI.js |
| 12.2 KiB | dist/toolbar/chunk-chunk-PIK3PADE.js |
Posted automatically by check-toolbar-size · sizes are toolbar output bytes (shipped, post-tree-shake) from the esbuild metafile
✅ Dist folder size — 🔺 +11.4 KiB (+0.0%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 1355.61 MiB · 🔺 +11.4 KiB (+0.0%)
⚠️ Playwright — 1 flaky
🎭 Playwright report · View test results →
- Preflight live mode (chromium)
These issues are not necessarily caused by your changes.
Annoyed by this section? Help fix flakies and failures and it will go green!
|
✅ Visual changes approved by @fercgomes — baseline updated in 22 changed. |
|
🔀 Tried to auto-resolve conflicts with I won't retry until the branch or master moves. |
Remove the "Starred" trigger from the left nav's browse tab and add usage instrumentation to the Files (project tree) feature. - Drop the Shortcuts/Starred entry from panelTriggerItems in NavTabBrowse. - Capture folder create, rename, delete, single and bulk move, search, sort change, and multi-select toggle in projectTreeLogic / ProjectTree, each tagged with the tree root so the Files tree can be isolated in analysis. Generated-By: PostHog Code Task-Id: 92284cb9-f964-4de0-88bd-49cb93f10f88
Address Greptile review findings on the files tree instrumentation: - Default the capture `root` to `project://` (not null) when a tree instance omits the prop, since the logic treats an absent root as the project root. Keeps interactions from those instances included when filtering on root: "project://". - Count only the moves actually issued in the bulk move, rather than the full checked count, so the event doesn't overstate the number of items moved when a selected folder has selected descendants. Generated-By: PostHog Code Task-Id: 92284cb9-f964-4de0-88bd-49cb93f10f88
22 updated Run: eca6184f-2dfa-43aa-a31f-31998a9acdd6 Co-authored-by: fercgomes <28491014+fercgomes@users.noreply.github.com>
c13894d to
42e2d31
Compare
2 participants
Problem
Closes GROW-217
Two asks came out of a Slack thread (linked below):
Changes
Shortcuts/ "Starred" entry frompanelTriggerItemsinNavTabBrowse, so it no longer renders in the browse tab. The underlying shortcuts logic and the "Add to starred" action on recent items are untouched.projectTreeLogicso it fires regardless of which UI path triggers the action, plus a couple of interaction-only events inProjectTree. Every event carries the treerootso the Files tree (project://) can be isolated from the other trees that reuse the same component.New events:
project tree folder created,project tree item renamed,project tree item deleted,project tree items moved(bulk),project tree item moved(drag),project tree searched,project tree sort changed,project tree multi-select toggled. These sit alongside the existingproject tree item clickedandproject tree folder toggled.Context: https://posthog.slack.com/archives/C0113360FFV/p1784832773192549
How did you test this code?
I (Claude) could not run the frontend toolchain in this environment (
node_modulesnot installed, so typecheck / lint / typegen did not run). Changes are limited to addingposthog.capturecalls and removing one array entry, following the existing capture patterns already in these files. No automated tests were added or run.🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Authored by Claude (PostHog Code). I located the Files feature (the ProjectTree / file-system panel) and the "Starred" nav trigger, then added the instrumentation in the kea logic rather than the component so events fire from any entry point. Care was taken to merge captures into existing
loadSearchResultsSuccessandmoveCheckedItemslisteners instead of declaring duplicate listener keys, which would have silently shadowed the real handlers. I was unable to read the linked Slack thread (auth-gated, no Slack tool available) and worked from the task description.Created with PostHog Code