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

Feature flags in toolbar frontend #5866

Merged
merged 79 commits into from
Sep 15, 2021
Merged

Conversation

rcmarron
Copy link
Contributor

@rcmarron rcmarron commented Sep 9, 2021

Changes

The front-end work to allow users to override their feature flags from the toolbar.

Dependent on #5824

feature-flag-demo.mov

Remaining items todo:

  • Tests
  • Get Posthog feature flags flowing into the toolbar to turn this feature on and off (super meta 🤔)

Checklist

  • All querysets/queries filter by Organization, by Team, and by User
  • Django backend tests
  • Jest frontend tests
  • Cypress end-to-end tests
  • Migrations are safe to run at scale (e.g. PostHog Cloud) – present proof if not obvious
  • New/changed UI is decent on smartphones (viewport width around 360px)

@timgl timgl temporarily deployed to posthog-pr-5866 September 14, 2021 21:44 Inactive
@timgl timgl temporarily deployed to posthog-pr-5866 September 14, 2021 22:44 Inactive
@timgl timgl temporarily deployed to posthog-pr-5866 September 14, 2021 22:48 Inactive
@timgl timgl temporarily deployed to posthog-pr-5866 September 14, 2021 22:50 Inactive
@timgl timgl temporarily deployed to posthog-pr-5866 September 14, 2021 22:55 Inactive
@rcmarron rcmarron force-pushed the feature-flags-in-toolbar-frontend branch from 738d746 to 75ddde6 Compare September 14, 2021 22:57
@timgl timgl temporarily deployed to posthog-pr-5866 September 14, 2021 22:58 Inactive
@timgl timgl temporarily deployed to posthog-pr-5866 September 14, 2021 23:21 Inactive
@rcmarron
Copy link
Contributor Author

rcmarron commented Sep 15, 2021

This is ready for review. A couple of notes:

  • This feature lives behind the feature flag posthog-toolbar-feature-flags, and can only be rolled out internally right now. This is because toolbar does not have access to Posthog's feature flags. Rather, the toolbar has access to the customer's feature flags, so right now, this feature will only work internally (because in that case, the "customer" is Posthog and we have the flag posthog-toolbar-feature-flags set). There is a bit of work to give the toolbar access to feature flags, and when we want to roll this out, we can either do that work or just remove the feature flag that it's behind.
  • The toolbar can definitely use a design pass beyond this feature. I tried to make the existing panels match the styles for the new feature flag panel, but it was a bit tricky. (see the pics below for where I landed) @clarkus I'd love your thoughts here.

  • Here is a quick demo of the feature
feature-flag-override-demo.mov

@rcmarron rcmarron marked this pull request as ready for review September 15, 2021 00:07
@clarkus
Copy link
Contributor

clarkus commented Sep 15, 2021

@rcmarron This looks good to me. I am tracking longer term toolbar improvements at #2370. If you had any specific items you think should be addressed, can you add them to that issue?

@timgl timgl temporarily deployed to posthog-pr-5866 September 15, 2021 00:17 Inactive
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.

This looks amazing!

I have just two comments regarding the nemesis of all macOS frontend developers: scrollbars 🤣 .

  1. The actions view has a scrollbar:
    2021-09-15 12 04 28

I actually recommend everyone working in frontend enable this setting to always catch them:

Screenshot 2021-09-15 at 12 05 41

The problem is all Windows computers always show scrollbars, so it'll look really ugly for those users... and we must prevent that.

  1. The scrollbar here is also slightly off. Could it be at the very right of the window?

image

@timgl timgl temporarily deployed to posthog-pr-5866 September 15, 2021 19:59 Inactive
@rcmarron rcmarron merged commit 3832ec7 into master Sep 15, 2021
@rcmarron rcmarron deleted the feature-flags-in-toolbar-frontend branch September 15, 2021 21:15
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.

6 participants