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

Definition Popups for all types of taxonomic filter groups! #8149

Merged
merged 25 commits into from
Feb 3, 2022

Conversation

alexkim205
Copy link
Contributor

@alexkim205 alexkim205 commented Jan 19, 2022

Changes

Mockups - https://www.figma.com/file/gQBj9YnNgD8YW4nBwCVLZf/PostHog-App?node-id=6782%3A34590

This PR implements the read only version of all different taxonomic filter group types. Hidden behind a feature flag and is an EE feature.

As of now clicking "View" doesn't do anything and the write hover card version will be implemented in a later PR.

Note: This isn't a picture perfect implementation of the mockup as there are some elements that are still unavailable for certain properties (i.e., last seen at, verified, etc.). As we unlock things in the backend, we'll show them in the frontend.

PS: Don't mind the large diff! Most of the changes come from renaming a few files and moving them around.

Demo

Screen.Recording.2022-02-01.at.6.54.10.PM.mov

Showcase

Screen Shot 2022-02-01 at 6 54 02 PM

Screen Shot 2022-02-01 at 6 53 59 PM

Screen Shot 2022-02-01 at 6 53 54 PM

Screen Shot 2022-02-01 at 6 53 50 PM

Screen Shot 2022-02-01 at 6 53 38 PM

Screen Shot 2022-02-01 at 6 53 29 PM

Screen Shot 2022-02-01 at 6 53 20 PM

Screen Shot 2022-02-01 at 6 53 08 PM

Screen Shot 2022-02-01 at 6 52 59 PM

Screen Shot 2022-02-01 at 6 52 56 PM

Screen Shot 2022-02-01 at 6 52 49 PM

Screen Shot 2022-02-01 at 6 52 42 PM

How did you test this code?

Manual QA of combinations of FF and EE license on and off.

@alexkim205 alexkim205 changed the title New definitions popup for taxonomic filter Definition Popups for all types of taxonomic filter groups! Jan 25, 2022
@alexkim205 alexkim205 self-assigned this Feb 2, 2022
@alexkim205 alexkim205 marked this pull request as ready for review February 2, 2022 03:04
Copy link
Contributor

@clarkus clarkus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a great start to me. Does the position logic for the card work the same as other tooltips and overlays? Just want to be sure it doesn't flow off the screen and positions in reasonable ways.

@alexkim205
Copy link
Contributor Author

Looks like a great start to me. Does the position logic for the card work the same as other tooltips and overlays? Just want to be sure it doesn't flow off the screen and positions in reasonable ways.

Yup, the card will always correct its position to the left or right of the filter depending on how much space is available on the screen. On smaller screens, we don't show the card at all which is the experience that is consistent with today's implementation.

Copy link
Collaborator

@mariusandra mariusandra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Made a few tweaks (e.g. disabled the view button), and let's get it in for testing.

children: React.ReactNode
}

// Wrapper
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not put this in the tag's name? :)

@mariusandra mariusandra enabled auto-merge (squash) February 3, 2022 14:18
@mariusandra mariusandra merged commit 91c73ae into master Feb 3, 2022
@mariusandra mariusandra deleted the feat/new-definition-popup branch February 3, 2022 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants