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

Show prompt to enable "fast mode" on large projects #378

Merged
merged 29 commits into from Apr 10, 2019

Conversation

Projects
None yet
4 participants
@rafeca
Copy link
Contributor

commented Apr 3, 2019

Description of the Change

This PR adds a prompt to the fuzzy finder which will inform users about the "fast mode" and will allow them to enable it easily.

In order to get the major benefits, the prompt will only be shown when opening large projects, and users will be able to disable the setting via the same prompt later.

Screen Shot 2019-04-03 at 19 54 49

Alternate Designs

More information in the associated task: #376

Next steps

Things that need to be implemented still:

  • Add logic to show the prompt only for large projects.
  • Add logic to modify the user settings when clicking the notification.
  • Add logic to remove the "new" label after some time (or after the prompt has been shown a specific number of times).
  • Add the prompt that allow users to disable the setting once it's enabled.
    • Add disable notification which allows to turn the setting off and show information to provide feedback.
    • Add logic to modify the user settings when clicking the notification.
  • Add metrics.
    • Number of times the "enable prompt" is shown.
    • Number of times people click on the "enable prompt".
    • Number of times people enable the fast mode from the notification.
    • Number of times the "disable prompt" is shown.
    • Number of times people click on the "disable prompt".
    • Number of times people disable the fast mode from the notification.
  • Improve codestyle.
  • Improve design.
  • Test the fuzzy-finder design updates in all UI themes that ship with Atom and also in the top ~3 community UI themes.
  • Improve prompt and notification copy.

Verification steps

  • Verify prompt with the default Atom config.
    • Check that no prompt is shown on small (< 10k files) projects.
    • Check that the enable prompt is shown on large (> 10k files) projects.
    • Check that the prompt is not shown at the same time than the following messages:
      • Re-indexing project
      • No results found.
    • Check that the prompt automatically appears after the project is reindexed.
    • Check that clicking the prompt shows a notification to enable fast mode.
      • Clicking on the cancel button of the notification does not do anything.
      • Clicking on the confirmation button of the notification changes the ripgrep and the scoring system config values of the fuzzy finder and shows a confirmation message.
  • Verify prompt when fast mode is enabled.
    • Check that now the prompt is shown on any project size.
    • Check that the message is different than the one to enable the prompt.
    • Check that the prompt is not shown at the same time than the following messages:
      • Re-indexing project
      • No results found.
    • Check that the prompt automatically appears after the project is reindexed.
    • Check that clicking the prompt shows a notification to disable fast mode.
      • Clicking on the cancel button of the notification does not do anything.
      • Clicking on the confirmation button of the notification changes the ripgrep and the scoring system config values of the fuzzy finder and shows a confirmation message.
  • Test the fuzzy-finder design updates in all UI themes that ship with Atom and also in the top ~3 community UI themes.
  • Verify that each of the newly created metric counters are sent correctly.

Applicable Issues

#376

Add prototype of prompt
This initial version only contains the basic logic to display the prompt
but does not handle when to show/hide it, different states or the logic
to change the settings yet.

Co-authored-by: Jason Rudolph <jason@jasonrudolph.com>
@rafeca

This comment has been minimized.

Copy link
Contributor Author

commented Apr 3, 2019

@jasonrudolph I'm gonna start working now on the first two items on the list:

  • Add logic to show the prompt only for large projects
  • Add logic to remove the "new" label after some time (or after the prompt has been shown a specific number of times).

rafeca added some commits Apr 3, 2019

Do not show the prompt when there are other messages
We don't want to show the prompt when other messages are displayed (like
the "jump to line" message or the "no results found" message).

In order to do so, we're only showing the prompt when the filter query
is empty.
@rafeca

This comment has been minimized.

Copy link
Contributor Author

commented Apr 3, 2019

@jasonrudolph for the "Add logic to show the prompt only for large projects." item I've decided to use the number of files as the threshold instead of the time to crawl: it's way easier to get this number from the view and even if using the time would be a better proxy to know if the user would benefit from the fast mode, the number of files is a good enough proxy IMO.

I've chosen 10K as the threshold, which is a little bit less than what we define as a medium project (more info). For this project size my maxed-out last-generation macbook pro takes ~4-5s to crawl so the impact of the fast mode will be already quite noticeable.

@rafeca

This comment has been minimized.

Copy link
Contributor Author

commented Apr 4, 2019

The main functionality and metrics are already done. This is a screencast of how the general flow looks like:

prompt

The next thing is to improve the messaging and tweak the design if needed, @jasonrudolph do you want to take over the PR for that?

/cc @simurai , @asheren if you want to weight in about the overall experience of the prompt (there's more context about this on #376).

@rafeca

This comment has been minimized.

Copy link
Contributor Author

commented Apr 4, 2019

My first initial thought is that we should add a confirmation message after enabling/disabling the fast mode... 😅

@jasonrudolph

This comment has been minimized.

Copy link
Member

commented Apr 4, 2019

This is coming together nicely! ⚡️

My first initial thought is that we should add a confirmation message after enabling/disabling the fast mode... 😅

I agree. 👍

The next thing is to improve the messaging and tweak the design if needed, @jasonrudolph do you want to take over the PR for that?

I'm happy to do that, but I want to be sure to wrap up my current work-in-progress first. With that in mind, it might be a few days before I can circle back around to this. So, if you want to push it forward in the meantime, feel free. Otherwise, I'll pick this up as soon as I tie up other loose ends. 🙂

@rafeca

This comment has been minimized.

Copy link
Contributor Author

commented Apr 4, 2019

I'm happy to do that, but I want to be sure to wrap up my current work-in-progress first. With that in mind, it might be a few days before I can circle back around to this. So, if you want to push it forward in the meantime, feel free. Otherwise, I'll pick this up as soon as I tie up other loose ends. 🙂

I think that's fine 😄 Thanks a lot!

@simurai
Copy link
Member

left a comment

Just based on the screenshot and gif.. looks great. 👍

Was wondering if it should be at the bottom of the list, so it's not the first thing you see every time. But should be fine as an introduction. Maybe later it could be moved.

jasonrudolph added some commits Apr 9, 2019

jasonrudolph added some commits Apr 9, 2019

Show resolved Hide resolved lib/experiment-prompt.js Outdated
@jasonrudolph

This comment has been minimized.

Copy link
Member

commented Apr 9, 2019

  • Improve design.
  • Improve prompt and notification copy.

@rafeca: I'm feeling pretty good about the design and the copy at this point. How are things looking to you? You can see an updated walkthrough of the prompts below...

demo

@50Wliu

This comment has been minimized.

Copy link
Member

commented Apr 10, 2019

Is there a reason the prompt is right-aligned?

@rafeca

This comment has been minimized.

Copy link
Contributor Author

commented Apr 10, 2019

@rafeca: I'm feeling pretty good about the design and the copy at this point. How are things looking to you? You can see an updated walkthrough of the prompts below...

Oh man, it looks much better! 😍I love the tone used on the messaging!

Is there a reason the prompt is right-aligned?

Initially we talked about showing it right-aligned to be able to display it at the same time that other messages (like the "re-indexing project..." one), but we finally added some logic to only show the prompt when there's no other message.

From my personal experience using a version of Atom with the left-aligned prompt on the fuzzy finder for a couple of days , when I search stuff my eyes go from the text input to the first result, passing through the prompt text every time causing (a very small) distraction.

By being on the right side, that text is no longer on the regular path of my vision (but it's still very noticeable the first time), which I think it's a better experience.

Of course, I'm not a designer and these are just my personal feelings 😅, so if there's a more technical or funded reason to keep it on the left side I'm ok changing it.

Was wondering if it should be at the bottom of the list, so it's not the first thing you see every time. But should be fine as an introduction. Maybe later it could be moved.

This would even be a better solution IMHO, unfortunately all the text placeholders available on atom-select-list (the component we use for the modal) show the text above the list, and we preferred to not modify that core component.

What do you think about the change that @jasonrudolph has just done? it moves the prompt to the right so it's much less "in your face".

@rafeca

This comment has been minimized.

Copy link
Contributor Author

commented Apr 10, 2019

I've added some verification plan to the issue which I'm going to follow. Something I realized while writing it is that since the prompt is only shown on large projects but the fast mode affects any project no matter the size, if somebody enables it while working on a large project but then experiences issues later when working on a smaller project they are not going to be able to disable it easier.

To solve this I'm going to display the opt-out prompt always, no matter the project size (this has the tradeoff of having more "noise" on small projects when the fast mode is enabled, but I think it's worth it).

I'm gonna add a config option to hide the prompt forever to solve this tradeoff.

@rafeca

This comment has been minimized.

Copy link
Contributor Author

commented Apr 10, 2019

I've tested the prompt across different themes and there's one of the top community themes that does not display it: nucleus-dark-ui.

Screen Shot 2019-04-10 at 12 36 14

The prompt is not shown but it causes some whitespace to appear between the text input and the list of results.

This is caused by an overlay div that the theme has, which also prevents any other message to appear on the fuzzy finder (like the "reindexing project...").

This is the 4th most downloaded UI theme, but it's no longer under development based on the Readme and it has similar issues on other packages.

We can either ignore this issue (it does not have a big impact IMO) or add some logic to disable the prompt if the user has this theme enabled.

@rafeca

This comment has been minimized.

Copy link
Contributor Author

commented Apr 10, 2019

We can either ignore this issue (it does not have a big impact IMO) or add some logic to disable the prompt if the user has this theme enabled.

Since it was really easy to do, I've added a check to disable the prompt for that theme

@jasonrudolph

This comment has been minimized.

Copy link
Member

commented Apr 10, 2019

I've added some verification plan to the issue which I'm going to follow.

@rafeca: Nice work. Thanks for doing that.

How confident are you that the metrics are being captured correctly? Should that be something we include in the verification plan?

@rafeca

This comment has been minimized.

Copy link
Contributor Author

commented Apr 10, 2019

How confident are you that the metrics are being captured correctly? Should that be something we include in the verification plan?

Ups, yeah I forgot to add this to the verification plan 😅

Regarding the metrics, we need to do some adjustments in our backend pipelines to handle the counters, I spoke with @telliott27 last week about it and he mentioned that the change would be very easy to do. @telliott27 do you need an issue to be created for that?

@rafeca

This comment has been minimized.

Copy link
Contributor Author

commented Apr 10, 2019

I've verified that the following measures are sent to our backend system after opening/closing the fuzzy finder a few times:

"measures": {
  "show-enable-prompt": 2,
  "click-enable-prompt": 2,
  "cancel-enable-prompt": 1,
  "confirm-enable-prompt": 1,
  "show-disable-prompt": 1,
  "click-disable-prompt": 3,
  "cancel-disable-prompt": 2,
  "confirm-disable-prompt": 1
}

Something worth to mention is that the show-enable-prompt and show-disable-prompt are counted every time the renderExperimentPrompt() method is called. Since there's some caching in the rendering logic of the fuzzy finder, when opening two times the fuzzy finder without interacting with it, it's going to count only a single event.

@rafeca rafeca changed the title WIP: Show prompt to enable "fast mode" on large projects Show prompt to enable "fast mode" on large projects Apr 10, 2019

@jasonrudolph jasonrudolph merged commit 44cf952 into master Apr 10, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jasonrudolph jasonrudolph deleted the fast-mode-prompt branch Apr 10, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.