Define and apply guidance for icon toggle states

[daviddossett]

Refs #162441

In some situations where a state is toggled (locking, pinning, hiding), we use a different approach for which icon is exposed. Sometimes we show the current state (e.g. locked) and sometimes we show the target state (currently unlocked, but the locked icon is displayed).

Ideally we can have reusable guidance to apply elsewhere.

[daviddossett]

Looking around, I see a couple of common situations where we are using icon buttons to toggle a state. Some first impressions:

1. It feels much clearer when we use the icon to represent the current state, not the target state, if we're swapping icons on click.
2. Even if we always do 1, the tooltip can and should still hint at the target state. E.g. the icon could show "Foo is locked" and the tooltip says "Unlock foo".

Thoughts?

Examples

Pinning

Locking

cc @bpasero @mjbvz @sandy081 The "locked" state representing autoscroll being turned on took me a few moments to really understand. I would have expected locked to mean that the scroll was locked in the current position. I think that's what you had before? Perhaps a more scroll-specific icon would be useful here.

This is a little different given that you click on the icon to toggle the state but it disappears instead of showing an unlocked icon. I think that's fine since it's still representing the current state.

Switching View Styles

cc @andreamah for feedback. This is in a branch if you're ok with this change and want me to make the changes:

[mjbvz]

Yes I agree that having the icon reflect the current state is easier to understand in cases like these. I think the fix for #162263 already aligned the output lock icon with this

[daviddossett]

@mjbvz based on what I saw in Insiders today, the locked icon meant that the scrolling was not locked.

Looking at this, I would expect the output to scroll. Once the lock icon was shown, I'd expect it to stay in place.

[andreamah]

@daviddossett so just to clarify, the toggle icons for search would be opposite of what it is currently? (ie: showing the current state with the icon rather than the target state)?

[daviddossett]

Basically:

• If you're looking at a flat list, we show the flat list icon
• If you're looking at a tree, we show the tree icon

This is to be consistent with other situations where we are toggling, e.g. locking, pinning, etc. We almost always show the current state and use the tooltip to indicate what will happen on click.

[mjbvz]

@daviddossett For me, locked should mean auto scrolling is on. The way I think about it, autoscrolling locks the viewport in place

Once you unlock it and turn auto scrolling off, you can scroll all over without VS Code trying to change the viewport

This is why I updated the icons in #162263

[andreamah]

I see it sort of like a play/pause button situation where the button shows the target state, but I also see where you're going with swapping them. Where else outside of our product is there a button that shows current rather than target state? I feel like I see target state more when I look around different products?

[daviddossett]

The way I think about it, autoscrolling locks the viewport in place

I see. I was thinking about it like we are locking the content in place, not the viewport. In any case, it took me more than a few seconds to realize what was going on. I also wonder if using the lock metaphor isn't quite right for autoscrolling in general.

I see it sort of like a play/pause button situation where the button shows the target state, but I also see where you're going with swapping them. Where else outside of our product is there a button that shows current rather than target state? I feel like I see target state more when I look around different products?

Play/pause is a valid example, but I feel like that's a special case compared to setting a view state. See the examples above for locking and pinning. It would be strange to have the "pinned" icon used for something that wasn't currently pinned. Or the same for locking. Some other examples:

This is slightly different because there's >2 options, but the current view state is represented by the icon.

Here's another example for locking and toggling visibility:

More than anything, I think I'm just generally confused when toggling the list states today. It's always the opposite of what I think it will be.

[andreamah]

Ah, I see. Yeah, it looks pretty natural in those settings, so I can see how that could make things less confusing. I give my 👍 for swapping the icons to show current state rather than target state.

[bpasero]

👍 for showing the current state in the icon for as long as that is consistently applied.

[Kroc]

As a devil's advocate UX argument; If the source-control list/tree button shows the current state, what result is a user expected to discern from this button by sight alone?

I believe this button in particular is a special case, it's not a toggle like a checkbox, it's a radio-button! It should really be presented as a choice of two icons with a selected state for the current, i.e.

[list] | tree

The thumbnail button in the bottom right of Explorer is basically the same principle:

It doesn't make sense in this context to show only one button regardless if you choose the current state or the resulting state as the icon because the current state does not convey what the button will do in a multiple-choice, non checkbox, context.

[eamodio]

I think I agree with having the icon show the current state and has generally been the model I've followed in GitLens.

But I think it gets more complicated with what you show as the tooltip. Does the tooltip also reflect the current state, or does it tell you what clicking on the button will do? I haven't really "picked a lane" on the tooltips -- sometimes I have them reflect the "to be taken" action and other times they reflect the current state.

I think it probably makes sense to be consistent and always show the action to be taken, so it follows the pattern of normal action buttons, but it can make it hard to understand what the icon you are looking at means. And I don't have a good solution to deal with both of those.

[Nahor]

The problem is that which icon should show depends on why one is looking at the icon at a given moment.
Sometimes, one wants to know what is the current state ("is the panel scroll-locked or not"), some other times, one wants to change the state ("where is the allow-scrolling button").

The best option is to have other cues beside the simple icon:

• in the multi-action case (like the icon/column/list/gallery mode), there is the "drop-down arrow", which indicates to the user that there will be a list of possible actions, and thus the icon can only represent the current state.
• with the eye icon next to an entry in a list, the entry itself is grayed out when not-visible
• in the same list example, the unlock icon is hidden unless one move the mouse cursor over the entry, while the locked icon remains always visible.
• in Apple UI, check-boxes change color (gray vs green) or is highlighted (bright/dark background instead of default background).
• play/pause is a bit special, but still has a cue: one can hear/see the music/video, so if one is looking for an icon, it's usually to change the state rather than checking what the current state is.

Ideally, the VSCode UI should add similar cues if possible, e.g.:

• change the background color of the "positive" icon, e.g. keep the unlock/unpinned icon as-is, but brighten/darken the background of the locked/pinned icon.
• change the icon when mousing over the icon, e.g. if the view is unlocked/unpinned, when mousing over, show the lock/pinned icon. The mouse-over indicate an intent to act, thus it can make sense to show the action/target state. However, this doesn't help for touch UX
• alternatively, when clicking the icon, instead of toggling the state, show a window with both options, similar to drop-down view. This does not fix the confusion about the current state, but it solves the confusion on the resulting state. This is also touch friendly at the cost of some extra clicks.