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

Review Preferences Sync UX #98820

Closed
texastoland opened this issue May 29, 2020 · 12 comments
Closed

Review Preferences Sync UX #98820

texastoland opened this issue May 29, 2020 · 12 comments
Assignees
Labels
settings-sync under-discussion Issue is under discussion for relevance, priority, approach

Comments

@texastoland
Copy link

texastoland commented May 29, 2020

  • Improve Sync views #93960 (comment): The menu item's label Preferences Sync is On looks like a toggle instead of opening a navigator. Preferences Sync (On)... seems straightforward.
  • Improve Sync views #93960 (comment): The menu doesn't access the sidebar which seems like a prominent use case. It's hidden in the command palette instead. None of the entries sound like Focus on Preferences Sync View.
  • Restore preferences from remote backups #97443 (comment): The cloud icon is subtle enough I couldn't find it. I understand why it's on the folder but kept looking at the file. I expected a context menu on the file itself.
  • There's no revert button in the diff view either.
  • The cloud icon didn't signify restoring to me. I kept trying the refresh button instead. Perhaps the refresh icon could be repurposed and its functionality moved to the top of the sidebar or even the command palette.
  • The Download... label confused me instead of Restore (no ... because it doesn't open a submenu).
@texastoland texastoland changed the title Clarify Preferences Sync interactions Review Preferences Sync UX May 30, 2020
@roblourens roblourens assigned sandy081 and unassigned roblourens May 31, 2020
@sandy081 sandy081 added settings-sync under-discussion Issue is under discussion for relevance, priority, approach labels Jun 2, 2020
@sandy081
Copy link
Member

sandy081 commented Jun 3, 2020

The menu doesn't access the sidebar which seems like a prominent use case. It's hidden in the command palette instead. None of the entries sound like Focus on Preferences Sync View.

I do not think this is a prominent use case. This is for administering or troubleshooting which is secondary and hence it is made accessible only through Command pallette.

Cloud icon / Download action / Restore state

I appreciate your feedback about download action that is to restore the state but since it is a brand new thing so I would wait and get more feedback before doing any changes over there

The menu item's label Preferences Sync is On looks like a toggle instead of opening a navigator. Preferences Sync (On)... seems straightforward.

I do not think this looks like a toggle button as there is no check mark associated to it. It is just an entry as other entries like Settings etc., which opens UI or actions related to the feature.

Since there is no action to be taken, I would be closing this. Thanks for the feedback.

@sandy081 sandy081 closed this as completed Jun 3, 2020
@sandy081 sandy081 removed the under-discussion Issue is under discussion for relevance, priority, approach label Jun 3, 2020
@texastoland
Copy link
Author

texastoland commented Jun 3, 2020

It's genuinely hard to use on multiple points. I don't understand how there are no action items. I identified easily implementable improvements. I literally couldn't find your new button despite seeing your screenshot. To argue a single point, where else in the entire UI do tree items not respond to context clicks?

@sandy081
Copy link
Member

sandy081 commented Jun 3, 2020

Context clicks are less discoverable than primary actions.

@texastoland
Copy link
Author

texastoland commented Jun 3, 2020

But your postulations are incorrect because I couldn't figure out your design. You're right about discoverability, but consistency is helpful when you can't figure out a UI. I really wanted a second opinion besides the original implementor. Is there a UX person free?

@sandy081
Copy link
Member

sandy081 commented Jun 3, 2020

Sure.

cc @misolori for opinions.

@miguelsolorio
Copy link
Contributor

The menu item's label Preferences Sync is On looks like a toggle instead of opening a navigator. Preferences Sync (On)... seems straightforward.

We modeled this after how browsers use the same terminology. For example, Chrome uses Sync is on and it links to their sync management (which is what we're doing here). Preferences Sync (On) is also a good alternative.

The menu doesn't access the sidebar which seems like a prominent use case. It's hidden in the command palette instead. None of the entries sound like Focus on Preferences Sync View.

I do think we should try to bubble this up in the Sync menu. We could have an item at the bottom (maybe it's a new section) that says Preferences Sync: Advanced, or something like that, to at least increase discoverability of this and other commands.

The cloud icon is subtle enough I couldn't find it. I understand why it's on the folder but kept looking at the file. I expected a context menu on the file itself.

I also agree with this, I would've expected a context menu on the settings.json and the parent item as well.

There's no revert button in the diff view either.

This seems like an oversight, we should add this.

The cloud icon didn't signify restoring to me. I kept trying the refresh button instead. Perhaps the refresh icon could be repurposed and its functionality moved to the top of the sidebar or even the command palette.

I do think we can find a better icon (possibly the history icon), however I don't think having it at the top makes sense as you need to choose what backup you want to restore to. It's much easier to go through the list and choose the one to backup to.

The Download... label confused me instead of Restore (no ... because it doesn't open a submenu).

+1

cc @Tyriar in case you have any additional thoughts

@texastoland
Copy link
Author

texastoland commented Jun 4, 2020

Sweet! I really appreciate @sandy081's implementation and @misolori's assessment. As someone OCD about their dotfiles I love being able to switch between past configurations. I just want it to be as obvious to everyone how useful this feature is.

@sandy081
Copy link
Member

sandy081 commented Jun 4, 2020

@texastoland Its good that you asked for third person's perspective.

We modeled this after how browsers use the same terminology. For example, Chrome uses Sync is on and it links to their sync management (which is what we're doing here). Preferences Sync (On) is also a good alternative.

Preferences Sync (On) might not be symmetric as we do not have Preferences Sync (Off). I would rather go with Preferences Sync is On or just Preferences Sync and prefer former as it is being used by other browsers too. Other suggestions are welcome.

I do think we should try to bubble this up in the Sync menu. We could have an item at the bottom (maybe it's a new section) that says Preferences Sync: Advanced, or something like that, to at least increase discoverability of this and other commands.

I see the point of discoverability but it is a trouble shooting feature (which is much advanced) and hence it is added only to command pallette. Let me know if everyone feel that this has to be a primary action, I can add it 👍

I also agree with this, I would've expected a context menu on the settings.json and the parent item as well.

I do not understand why we need same action as primary and also as secondary. Do we follow this anywhere else?

BTW this action was added only to parent because the parent can contain more files for example, different snippets files and in this case adding a download/revert action is specific to that file which is a different feature . It would be same if we add more settings files in future (say platform specific settings). Hence I am reluctant to add same action there and it is a different feature.

There's no revert button in the diff view either.

Agreed that this is good to have - Filed separate request for this #99340

I do think we can find a better icon (possibly the history icon), however I don't think having it at the top makes sense as you need to choose what backup you want to restore to. It's much easier to go through the list and choose the one to backup to.

Not sure history icon is better as it does not mean that it is downloading and replacing. I chose download as it is closer to what we are doing here - downloading the state and replacing. I am open to replace with a better icon.

The Download... label confused me instead of Restore (no ... because it doesn't open a submenu).

Agreed. Fixed.

Reopening to continue the discussion.

@sandy081 sandy081 reopened this Jun 4, 2020
@sandy081 sandy081 added the under-discussion Issue is under discussion for relevance, priority, approach label Jun 4, 2020
sandy081 added a commit that referenced this issue Jun 4, 2020
@miguelsolorio
Copy link
Contributor

I do not understand why we need same action as primary and also as secondary. Do we follow this anywhere else?

It's common to go to an individual item to perform a single action, just like in SCM. You can make an action on a single one (stage one) or the entire parent (stage all). So I wouldn't say it's the same action since there could be additional files in other views. So you can either restore a single file or the entire folder. That's my expected UX, that may not be what happens in the background.

Not sure history icon is better as it does not mean that it is downloading and replacing. I chose download as it is closer to what we are doing here - downloading the state and replacing. I am open to replace with a better icon.

I don't think users think of this as downloading but rather restoring from a backup. That's the terminology used in backups and this is called "Backups". As I said earlier, the user doesn't see this as "downloading" even if that is what is happening in the background.

Btw, in this commit (5b068c1) you changed the terminology to "Revert", I think you meant "Restore"?

@miguelsolorio
Copy link
Contributor

Also, why do we show the a line item for the machine that has no actions? This seems like a decoration/label that can be added to the end of the file

image

@sandy081
Copy link
Member

sandy081 commented Jun 4, 2020

It's common to go to an individual item to perform a single action, just like in SCM. You can make an action on a single one (stage one) or the entire parent (stage all). So I wouldn't say it's the same action since there could be additional files in other views. So you can either restore a single file or the entire folder. That's my expected UX, that may not be what happens in the background.

Convinced - Filed #99401

Regarding icon - I agreed that cloud-download icon is not the best, but felt better than history icon. Would be happy to get a better icon here.

Btw, in this commit (5b068c1) you changed the terminology to "Revert", I think you meant "Restore"?

Sorry. Changed to Restore.

Also, why do we show the a line item for the machine that has no actions? This seems like a decoration/label that can be added to the end of the file

This is to see from which machine this request is coming from without opening the file.

@sandy081
Copy link
Member

Closing this as there are issues tracking individual requests.

@github-actions github-actions bot locked and limited conversation to collaborators Sep 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
settings-sync under-discussion Issue is under discussion for relevance, priority, approach
Projects
None yet
Development

No branches or pull requests

4 participants