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.