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

Update the logged out status bar item with more obvious CTA #3230

Merged
merged 16 commits into from
Mar 17, 2024

Conversation

toolmantim
Copy link
Contributor

@toolmantim toolmantim commented Feb 21, 2024

Updates the Cody status bar button to have the text "Sign in to Cody" when not logged in.

PRD

Asana task

Before:

CleanShot.2024-02-21.at.11.43.34.mp4

After:

CleanShot.2024-03-15.at.23.45.55.mp4

Events added:

  • CodyVSCodeExtension:signInNotification:shown
  • CodyVSCodeExtension:signInNotification:signIn:clicked
  • CodyVSCodeExtension:signInNotification:doNotShow:clicked
  • CodyVSCodeExtension:statusBarIcon:clicked, { loggedIn: boolean }

Test plan

  • CI
  • Test logged out & logging in
  • Test logged in

@toolmantim
Copy link
Contributor Author

This doesn't update the setup notification, but it uses different language that we should probably also update:

CleanShot 2024-02-21 at 11 32 38@2x

@chenkc805
Copy link

Thanks Tim! Not sure if this is in progress, but just so we don't miss it:

  • Can we update the copy in the popup? "Setup" isn't quite the right call to action. LMK if this is in a separate PR
  • Is there a way to have the status bar icon open up the sidebar instead of the quickpick if you're not logged in? Gets rid of the user having to see the quickpick, thus removing a step.

@toolmantim
Copy link
Contributor Author

Yep we can update the copy, and I can make it just focus the sidebar.

It does mean the settings quickpick items (links to support/docs) will only be accessible if you’re logged in. I think that’s fine, but just wanted to call it out.

@chenkc805
Copy link

It does mean the settings quickpick items (links to support/docs) will only be accessible if you’re logged in. I think that’s fine, but just wanted to call it out.

Yes that's right. Most of the information here is only relevant to logged in users, and if users want to look for docs they can go to Google.
Screenshot 2024-02-21 at 4 25 39 PM

@toolmantim toolmantim force-pushed the tl/improve-login-statusbar-cta branch from 992ae9b to 38a2cff Compare March 14, 2024 09:58
@toolmantim toolmantim force-pushed the tl/improve-login-statusbar-cta branch from 15cd410 to 4e9cfad Compare March 15, 2024 11:39
@toolmantim toolmantim marked this pull request as ready for review March 15, 2024 12:47
@toolmantim
Copy link
Contributor Author

This PR is ready to go.

@kelsey-brown the final events we ended up logging are slightly different to the Google Doc:

  • CodyVSCodeExtension:signInNotification:shown
  • CodyVSCodeExtension:signInNotification:signIn:clicked
  • CodyVSCodeExtension:signInNotification:doNotShow:clicked
  • CodyVSCodeExtension:statusBarIcon:clicked, { loggedIn: boolean }

Are these okay? In particular I'm wondering if the loggedIn properly on statusBarIcon clicked can be used to identify people clicking the status bar when loggedIn == false.

@toolmantim toolmantim requested a review from a team March 15, 2024 13:06
@kelsey-brown
Copy link

@toolmantim is CodyVSCodeExtension:signInNotification:shown the enrollment event then?

In particular I'm wondering if the loggedIn properly on statusBarIcon clicked can be used to identify people clicking the status bar when loggedIn == false.

Yes, if this is logged with the metadata on the event we can filter/pivot clicks by loggedIn = true / loggedIn = false

@toolmantim
Copy link
Contributor Author

Thanks @kelsey-brown. Yep, that should work as the enrollment event. Though this isn't running as an experiment in the code at all (I've never done that before)

Copy link
Member

@sqs sqs left a comment

Choose a reason for hiding this comment

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

For the actual text, how about: “(icon) Sign In”? That seems best. Shortest and least words.

I hate long and advertisingy status bar items. Other competitors (won’t name in public) do this and it looks very spammy. This is not spammy but I want to really be careful to have a super short message down here.

@toolmantim
Copy link
Contributor Author

toolmantim commented Mar 15, 2024

@sqs I didn't want to confuse it with a Microsoft/GH login button. Are you thinking it's a bad experience for the person that just installed it, or for the person that keeps it installed but doesn't login and wants to ignore it for now (and we don't want them to uninstall)?

@sqs
Copy link
Member

sqs commented Mar 16, 2024

@sqs I didn't want to confuse it with a Microsoft/GH login button. Are you thinking it's a bad experience for the person that just installed it, or for the person that keeps it installed but doesn't login and wants to ignore it for now (and we don't want them to uninstall)?

Both. And I just have a negative reaction to anything that takes up a lot of space in my status bar. Can you check with a few other devs on the team for their kneejerk reaction? If I'm the only one, then feel free to disregard.

@toolmantim
Copy link
Contributor Author

First impressions matter! Will straw poll.

@toolmantim
Copy link
Contributor Author

I've updated the label to be "Sign In" so we can unblock this and get it out

@toolmantim toolmantim requested review from a team and sqs and removed request for a team March 17, 2024 22:20
@toolmantim toolmantim merged commit 5a94b6e into main Mar 17, 2024
16 checks passed
@toolmantim toolmantim deleted the tl/improve-login-statusbar-cta branch March 17, 2024 22:51
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.

None yet

4 participants