feat(cohorts): add cohort filter grammars#9540
Conversation
alexkim205
changed the title
There was a problem hiding this comment.
Looks super good. I noticed there are still some TODO items in storybook, so maybe there is still more to review?. I saw a couple of things we could align.
When using the property criteria, the picker still references "events".
The event / property picker here feels less actionable than all the other inputs. Since it's really just a more-capable selection control, can we give it the same button-like treatment you've set for the other components? TL;DR can we make the text on the event picker blue, too? Or make them all using the standard black... I can see benefits to both options, but the core concern is that they all look the same here.
|
Thanks @clarkus, addressed!
|
|
Thanks @clarkus, addressed both concerns in this revision. This PR is now ready for a review! I left one TODO for the property value selector, which depends on the row data logic implementation. I'll leave that for another PR. |
|
Merging this in to unblock frontend logic work. Pretty safe since it doesn't touch any components in prod. |
* master: (137 commits) feat(cohorts): add cohort filter grammars (#9540) feat(cohorts): Backwards compatibility of groups and properties (#9462) perf(ingestion): unsubscribe from buffer topic while no events are produced to it (#9556) fix: Fix `Loading` positioning and `LemonButton` disabled state (#9554) test: Speed up backend tests (#9289) fix: LemonSpacer -> LemonDivider (#9549) feat(funnels): Highlight significant deviations in new funnel viz (#9536) docs(storybook): Lemon UI (#9426) feat: add support for list of teams to enable the conversion buffer for (#9542) chore(onboarding): cleanup framework grid experiment (#9527) fix(signup): domain provisioning on cloud (#9515) chore: split out async migrations ci (#9539) feat(ingestion): enable json ingestion for self-hosted by default (#9448) feat(cohort): add all cohort filter selectors to Storybook (#9492) feat(ingestion): conversion events buffer consumer (#9432) ci(run-backend-tests): remove CH version default (#9532) feat: Add person info to events (#9404) feat(ingestion): produce to buffer partitioned by team_id:distinct_id (#9518) fix: bring latest_migrations.manifest up to date (#9525) chore: removes unused feature flag (#9529) ...
2 participants
Problem
Adds filter grammars as part of cohort filtering project.
Design doc: https://docs.google.com/document/d/1fxgTkPLPzNGIlODCqOof3yc-5qqxPTDY3kgtQe_koa4/edit
Changes
This PR is only responsible for implementing a foundational grammar for cohort filtering on the frontend. For this review, please ignore TODO's and styling.
P.S: Also note that the AND/OR structure isn't refactored yet so what you see here isn't completely 1:1 with the mockups (the behavioral filter will be brought up into a separate section headline).
PPS: The diff ballooned because I ended up refactoring a lot of the existing selector components from #9492. Static objects in
constants.tsxalso make up new ~700 lines as well.Screen.Recording.2022-04-26.at.3.35.18.PM.mov
https://www.figma.com/file/gQBj9YnNgD8YW4nBwCVLZf/PostHog-App?node-id=8983%3A41658
👉 Stay up-to-date with PostHog coding conventions for a smoother review.
How did you test this code?
Storybook