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

Add command palette badge to new TopNavigation #2678

Merged
merged 3 commits into from
Dec 8, 2020
Merged

Conversation

Twixes
Copy link
Collaborator

@Twixes Twixes commented Dec 5, 2020

Changes

This adds a command palette badge to TopNavigation.tsx, which works much like the button in the old nav. Resolves #2645.

Toggle

Also some tiny typing fixes.

@timgl timgl temporarily deployed to posthog-new-nav-palette-ld9qul December 5, 2020 01:12 Inactive
Copy link
Contributor

@paolodamico paolodamico left a comment

Choose a reason for hiding this comment

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

I'm not 100% on board with this way of showing the command palette. Mainly because the badges at the top are meant as status badges, i.e. they do not pull attention on the regular course of business. Also, the gray doesn't really communicate that the element is clickable. In addition, this isn't particularly emphatic about the command palette.

The above being said, I'm not opposed to merging this for now.

@Twixes
Copy link
Collaborator Author

Twixes commented Dec 7, 2020

That seems like the best place for such a thing though IMO, akin to the macOS menu bar. Though definitely not prominent with this color. How about our primary blue?

Badge --primary

I think the button we had in the old nav was ideal, with the keyboard shortcut shown, but that's harder to pull off in a way consistent with the new style.

@paolodamico
Copy link
Contributor

I like it! Still think at some point in the near future we might come up with something different (particularly if we improve the search capabilities, we can consider changing to a search bar), but for now it's 👌👌, I've updated and merged master for cypress tests to pass.

@timgl timgl temporarily deployed to posthog-new-nav-palette-ld9qul December 7, 2020 14:33 Inactive
@Twixes Twixes merged commit 0de0e25 into master Dec 8, 2020
@Twixes Twixes deleted the new-nav-palette branch December 8, 2020 06:26
@Twixes Twixes removed the request for review from jamesefhawkins December 8, 2020 06:26
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.

New UX doesn't signify the command palette exists
3 participants