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

Swap list/tree icons to reflect current state instead of target state #164835

Merged
merged 2 commits into from
Oct 28, 2022

Conversation

daviddossett
Copy link
Contributor

Ref #162595

Swaps flat list and tree list icons in the Search and SCM views to represent what you're currently looking at instead of what you can toggle to.

CleanShot.2022-10-27.at.16.47.16.mp4

cc @andreamah @lszomoru

@roblourens
Copy link
Member

I prefer the other way but ok :(

@roblourens
Copy link
Member

I looked at the examples in the thread and I definitely agree with you on the other cases, like "pinned" should the current state, so I don't know why I'm inconsistent.

@aeschli aeschli modified the milestones: October 2022, November 2022 Oct 28, 2022
@andreamah
Copy link
Contributor

It is a little tricky, since stuff like the collapse button still shows target state. Could it be that it's inline with other icons that show target state that throws you off? @roblourens

@daviddossett
Copy link
Contributor Author

the collapse button still shows target state.

That's a good point. For whatever reason that feels more like a command rather than a UI state. That is to say, it might be strange to show the UI saying "you're currently looking at a collapsed list". Maybe because it's less of a formal view state and more of a cleanup utility of sorts?

@andreamah
Copy link
Contributor

True. Have we considered using something like the case-sensitive highlight in find widgets?

image

So rather than toggling the icon, we're essentially either in "tree mode on" or "tree mode off"

@daviddossett
Copy link
Contributor Author

I was thinking about that too. I'm wondering if we use those boolean toggles outside of text inputs anywhere. I can't think of any off the top of my head.

@andreamah
Copy link
Contributor

Yeah, I think I looked into this when I created #162088. It didn't seem like there was anywhere I could find, but I think it's a clear way to show a toggle and we should maybe have it in more places (?)

@daviddossett
Copy link
Contributor Author

My only concern is that they seem better suited for something that is on or off rather than two descriptive non-boolean states. Based on the feedback in the UX channel I'll merge this as a starting point and we can see how it feels. Happy to continue iterating on this if we sit with it for a while and it doesn't feel right.

@daviddossett daviddossett merged commit 77381ab into main Oct 28, 2022
@daviddossett daviddossett deleted the ddossett/normal-parakeet branch October 28, 2022 18:46
@github-actions github-actions bot locked and limited conversation to collaborators Dec 12, 2022
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.

None yet

6 participants