Skip to content
This repository was archived by the owner on Nov 6, 2018. It is now read-only.

feat: improve UX of extension add/enable/remove#50

Merged
sqs merged 1 commit intomasterfrom
add-ui
Sep 22, 2018
Merged

feat: improve UX of extension add/enable/remove#50
sqs merged 1 commit intomasterfrom
add-ui

Conversation

@sqs
Copy link
Member

@sqs sqs commented Sep 22, 2018

Simplifies what a user needs to know/do to use extensions, specifically the add/remove/enable/disable operations. The set of possible states for an extension remain the same, but the UI actions exposed to the user are much simpler and only address the common cases. To enable/disable/add/remove extensions for everyone (in global settings) or for organization members (in organization settings), you must edit those settings manually. That is acceptable (for now, at least) and makes the UI much simpler.

The previous approach is best described as "show the user the set of all possible changes to settings related to this extension".

The new approach is "make it easy for the user to start or stop using an extension, and to know why an extension is enabled if it's enabled". The reason the latter is important (knowing why an extension is enabled) is that many extensions will be enabled for the user in global or organization settings even before they've taken explicit action.

See https://sourcegraph.slack.com/archives/CCLF4R6EM/p1537634102000100?thread_ts=1537605588.000100&cid=CCLF4R6EM (internal link) for more context.


screenshot from 2018-09-22 12-51-58

screenshot from 2018-09-22 12-50-41

screenshot from 2018-09-22 12-45-40

screenshot from 2018-09-22 12-45-28

screenshot from 2018-09-22 12-45-15

screenshot from 2018-09-22 12-44-52

screenshot from 2018-09-22 12-44-45


screenshot from 2018-09-22 13-08-08


screenshot from 2018-09-22 13-08-00

Simplifies what a user needs to know/do to use extensions, specifically the add/remove/enable/disable operations. The set of possible states for an extension remain the same, but the UI actions exposed to the user are much simpler and only address the common cases. To enable/disable/add/remove extensions for everyone (in global settings) or for organization members (in organization settings), you must edit those settings manually. That is acceptable (for now, at least) and makes the UI much simpler.

The previous approach is best described as "show the user the set of all possible changes to settings related to this extension". The new approach is "make it easy for the user to start or stop using an extension, and to know why an extension is enabled if it's enabled".

See https://sourcegraph.slack.com/archives/CCLF4R6EM/p1537634102000100?thread_ts=1537605588.000100&cid=CCLF4R6EM (internal link) for more context.
@sqs sqs requested review from chrismwendt and emidoots September 22, 2018 19:58
@codecov
Copy link

codecov bot commented Sep 22, 2018

Codecov Report

Merging #50 into master will decrease coverage by 2.33%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #50      +/-   ##
==========================================
- Coverage   18.38%   16.05%   -2.34%     
==========================================
  Files          36       36              
  Lines        1164     1115      -49     
  Branches      262      250      -12     
==========================================
- Hits          214      179      -35     
+ Misses        950      936      -14
Impacted Files Coverage Δ
src/extensions/extension.ts 0% <ø> (-80%) ⬇️
src/context.ts 0% <ø> (ø) ⬆️
...rc/extensions/ExtensionConfigureButtonDropdown.tsx 0% <0%> (ø)
src/extensions/ExtensionAddButton.tsx 0% <0%> (ø)
src/extensions/manager/ExtensionCard.tsx 0% <0%> (ø) ⬆️
src/extensions/ExtensionPrimaryActionButton.tsx 0% <0%> (ø)
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1ce6cdc...dabe34d. Read the comment docs.

@sqs sqs merged commit 7f1d38c into master Sep 22, 2018
@sqs sqs deleted the add-ui branch September 22, 2018 20:08
@sourcegraph-bot
Copy link

🎉 This PR is included in version 8.6.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@emidoots
Copy link
Member

Thanks for taking a stab at this.. but I am struggling to see this as a good change. I currently view this change as a regression overall. Here's why:

(1) You have removed the ability to add the extension globally / just for an org.

  • This part of the UX was not confusing to me at all, in fact it worked exactly as I expected it to.
  • This is a major drawback for the code discussions extension:
    a. This means I will now need to include instructions in the README describing how to add these settings to your global settings, because code discussions is effectively useless when enabled for only one user.
    b. There will likely be user confusion about "why are global and site settings different", or "I added it to my site settings and it doesn't work. Wait, they're different? What?!".
  • I believe this will be a major drawback not just for code discussions, but also for all other extensions as well. The common case is not "I as a single user wish to have an extension" but rather "my team uses Sourcegraph, and I want them to have this extension available to them".

In fact, I would argue that if any individual settings modes should be removed it should be the user and organization-level ones, not the global one.

(2) You have not addressed my confusion as a user over why enable/disable is different from add/remove

In this screenshot:


image


If I want to get rid of the extension, I don't understand whether or not I should be clicking disable or remove. Having both options immediately makes me ask questions like:

  • Does disable not fully get rid of the extension? Will it still have access to my code or something weird in the background?
  • Does remove destroy data that the extension created? If I decide to add the extension back later, will clicking remove make that harder in the future than disable?
  • The UX doesn't answer these questions for me, and doesn't communicate the difference to me at all.

@sqs
Copy link
Member Author

sqs commented Sep 23, 2018

Thanks for the feedback. I will think about the first point in a bit.

For the 2nd point, how could we convey these here? What if we just remove the “remove” action? Do you have this same confusion about VS Code extensions (which have the same options)?

@sqs
Copy link
Member Author

sqs commented Sep 23, 2018

Re: org and global settings, what do you mean by “why are global and site settings different?” and “I added it to my site settings and it doesn’t work. Wait, they’re different?”

If you add it to global settings, it will show as added and enabled for every user—did you think it would not work that way? Or is the confusion about global settings vs site config?

As for needing this to be smooth for code discussions, I see that and wasn’t thinking about that. I’ll think of a simple way to add that functionality back so it’s just as easy as it was before to add an extension for everyone/an organization.

@sqs
Copy link
Member Author

sqs commented Sep 23, 2018

For (2), also do you have those same questions about Chrome extensions? How did you clear up your understanding there? What UI cues etc helped you?

@emidoots
Copy link
Member

For the 2nd point, how could we convey these here? What if we just remove the “remove” action? Do you have this same confusion about VS Code extensions (which have the same options)?

I think just removing the "remove" action or "disable" action entirely would be good.

VS Code provides me these options:

image

This conveys to me two bits of information:

  1. There is per-workspace disablement, so global disablement may just exist for that reason.
  2. Because it is a local editor environment, I realize the extension may install e.g. additional language tooling that it needs and by clicking uninstall that is what I would be losing. In the case of Sourcegraph, there is no logical explanation for me like "the extension may install additional tools to your system" because everything is hosted, so the distinction between remove and disable becomes non-obvious.

Re: org and global settings, what do you mean by “why are global and site settings different?” and “I added it to my site settings and it doesn’t work. Wait, they’re different?”

If you add it to global settings, it will show as added and enabled for every user—did you think it would not work that way? Or is the confusion about global settings vs site config?

As for needing this to be smooth for code discussions, I see that and wasn’t thinking about that. I’ll think of a simple way to add that functionality back so it’s just as easy as it was before to add an extension for everyone/an organization.

By these statements I just meant to convey the general issues around me describing to users in a README how to enable code discussions' extension for all users of a Sourcegraph instance. So, to be clear:

  • I do think it will work that way ("If you add it to global settings, it will show as added and enabled for every user").
  • The confusion is about global settings vs site config.

For example, if the code discussions README says:

  1. Visit your global Sourcegraph settings.
  2. Add extensions: [...] and save.

It is very easy for even experienced Sourcegraph users to think "Oh, I know where global Sourcegraph settings are -- they are in my site config!" and add the snippet of code to their site config which will fail because it's not their global settings. The reason this is a problem is because users generally don't interact with the global settings.

When the ability to configure this via UI buttons existed (before this PR) this was a non-issue because users just wouldn't run into the question in the first place: they would just use the UI.


For (2), also do you have those same questions about Chrome extensions? How did you clear up your understanding there? What UI cues etc helped you?

I don't have the same questions about Chrome's UI, which is interesting. I think the reason that I don't is because:

chrome://extensions/ shows me my installed extensions only, I can't add more extensions from there (or at least, the UI is not tailored for that). So if I click Remove I understand it goes away forever and I will have to find it on the Chrome web store again if I decide to use it again in the future.

In fact, the Chrome Webstore itself offers me none of these options. If I visit an extension like Sourcegraph that I already have installed, I have no actions. I cannot remove it, disable it, etc. from this page.


To conclude my thoughts here, I think the best UX here would be:

(0) Add back the ability to enable extensions at a global level from the UX (even if just for the sake of code discussions).
(1) If I don't have an extension installed, I can Add it. (only option).
(2) If I have an extension installed, I can Disable it if it is added or enabled.
(3) If I have an extension installed but it is disabled, I can Enable it.
(4) To remove the extension fully, I have to remove it from the config file manually. (AFAIK removing extension has no real difference to users today aside from reference to it being removed from their config)

My suggestion for (4) would change if:

a. We offered "disable for workspace" in the same UI location like VS Code does (this wouldn't be possible unless our extensions UX was tied to having a workspace open).
b. We offer a separate page for viewing your installed extensions. Like Chrome, you could not manage installed extensions from the "store page" (but a link could bring you to the management page). It would then be obvious that removing an extension means "it goes away from my installed extensions page and I'll have to find it in the store again if I do that".

@chrismwendt
Copy link

chrismwendt commented Sep 27, 2018

While bumping e-c-c to support diff pages in the browser extension, I noticed that the extension list add/remove buttons are completely gone due to this change. I'm working on figuring out how to support client settings again, since this PR prevents that from working.

Edit Fixed by editing the highest precedence settings (rather than always User settings).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants