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

Global hotkey navigation #3740

Merged
merged 8 commits into from Mar 25, 2021
Merged

Global hotkey navigation #3740

merged 8 commits into from Mar 25, 2021

Conversation

paolodamico
Copy link
Contributor

@paolodamico paolodamico commented Mar 24, 2021

Changes

This PR introduces global hotkey navigation to make our overall experience smoother for power users. Everything is behind feature flag hotkeys-3740.

  • Closes Test hotkey support to improve UX #2907.
  • Allows navigating to any of the scenes accessed through the main navigation (press "G" anywhere to try out).
    image
  • Allows navigating between the insights views (fully supports lifecycle & stickiness if remove-shownas is enabled).
  • Introduces a tooltip description to menu items to increase feature discoverability. This is also the basis for global keyboard navigation discoverability.

In addition this PR:

  • Converts Insights.tsx to Typescript and fixes a bunch of Typescript errors.
  • Standardizes and makes styles for hotkeys global.
  • Usage is fully instrumented.

Checklist

  • All querysets/queries filter by Organization, by Team, and by User
  • Django backend tests
  • Jest frontend tests
  • Cypress end-to-end tests

@timgl timgl temporarily deployed to posthog-pr-3740 March 24, 2021 22:51 Inactive
@timgl timgl temporarily deployed to posthog-pr-3740 March 24, 2021 23:42 Inactive
@timgl timgl temporarily deployed to posthog-pr-3740 March 25, 2021 01:40 Inactive
@paolodamico
Copy link
Contributor Author

I would love to get particularly input on both the concept of the tooltip message for the navigation bar items, and the actual copy. Want to make sure it's helpful and not intrusive.

@paolodamico paolodamico marked this pull request as ready for review March 25, 2021 01:49
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.

One feature flag check bug and some thoughts.

frontend/src/layout/navigation/MainNavigation.tsx Outdated Show resolved Hide resolved
frontend/src/lib/hooks/useKeyboardHotkeys.tsx Show resolved Hide resolved
frontend/src/lib/hooks/useKeyboardHotkeys.tsx Show resolved Hide resolved
frontend/src/lib/hooks/useKeyboardHotkeys.tsx Outdated Show resolved Hide resolved
frontend/src/scenes/insights/Insights.tsx Show resolved Hide resolved
frontend/src/scenes/insights/Insights.tsx Show resolved Hide resolved
@paolodamico
Copy link
Contributor Author

Alright all feedback addressed, ready for a final look.

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!

@corywatilo
Copy link
Contributor

I was playing around with a way to make them a little more subtle in the nav tooltips. (This eliminated the need for the "Keyboard navigation" label.)

image

As for the prominence for the analysis views, I sort of agree with Marius. I did a quick version to see if we can make them blend a bit more and here was my attempt:

image

@timgl timgl temporarily deployed to posthog-pr-3740 March 25, 2021 17:45 Inactive
@paolodamico paolodamico merged commit f4a87ef into master Mar 25, 2021
@paolodamico paolodamico deleted the global-hotkeys-I branch March 25, 2021 18:04
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.

Test hotkey support to improve UX
4 participants