microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
164.86k stars 29.51k forks source link

responsive GUI regarding `search.smartCase` #206913

Open PabloLION opened 8 months ago

PabloLION commented 8 months ago

Introduction

I'd like to present several proposals regarding the representation of search.smartCase within the "match case" button interface. These proposals aim to enhance user experience by providing clearer visual cues about the current search mode. The options include:

Background

Since its inception in January 2018 (version 1.20), search.smartCase has been a significant feature. However, its subtle operation often leaves users unaware of its activation, leading to confusion. This issue has been echoed in multiple reports:

Issue Statement

Upon reviewing the design mentioned in #42687, it appears its full potential has not been realized in the current implementation. The design proposed four distinct states to signify the interaction between the user's input and the "match case" functionality. original-design state A: searching with all lowercase with "match case" off state B: searching with uppercase, with "match case" off -> search.smartCase is activated, which turns on "match case" in a non-sticky way. state C: searching with uppercase, from state B, a click turns off "match case". By my assumption, this remembers "smart case" is off for this search, to distinguish from state A. state D: searching with uppercase, from state C, a click turns on "match case", in a sticky way.

However, my experience, especially with version 1.87.0-insider at commit e73419bef379f7956f024557cbf40bd3755a0645, suggests a deviation from these intended states, particularly never encountering the transitional "smart case" activation state. This deviation might be tolerable if intended as a simplification, yet it inadvertently complicates user interaction and understanding, especially concerning the elusive state C.

Proposals for Enhancement

Minimal Modification: Concealing the "Match Case" Button

Given the current functionality of search.smartCase:

  1. Ignoring case with entirely lowercase input.
  2. Searching exclusively in lowercase with lowercase input.
  3. Conducting a case-sensitive search with any uppercase input, rendering the "match case" button superfluous.

This proposal suggests hiding the "match case" button when search.smartCase activates with uppercase input. This adjustment would subtly inform advanced users of the feature's activation, simplifying the interface.

Original Design Fulfillment: Comprehensive Implementation of #42687

Adhering more closely to the original design would enhance the graphical user interface by vividly demonstrating the state of search.smartCase. This approach may require additional development to align with the current behavior of search.smartCase, but it promises a more intuitive user experience by clarifying the logic behind state transitions, especially state C.

Ternary State Introduction: A New Dimension for the "Match Case" Button

Considering the ambiguity surrounding state C in the original proposal, introducing a ternary state ("on", "off", and "smart case") for the "match case" button could provide a clearer representation of the search's current state. This approach, while more complex, enriches the user interface by indicating the active mode of search.smartCase through distinctive icons (e.g., hollow/solid stars for "smart case" activation). smart-match-case (Designing the icon is hard for me, this is just the idea)

Further Discussion and Contribution

I invite @roblourens and @octref to engage in further discussion about this topic, particularly around ripgrep integration and optimal representations of case sensitivity in search functionalities. Additionally, I've prepared a flowchart to facilitate understanding of the proposed "smart case" and "match case" functionalities and I added a flowchart to help understand the logic of the "smart case" and "match case" button. image

I am eager to contribute to either the Minimal Modification or Ternary State proposals through a pull request

PabloLION commented 8 months ago

To be more specific on the "sparkling stars" I wanted to use something like the copilot ones. Here's a screen shot (not sure about the copy right) image

Also there's an older version of the issue statement before ChatGPT's editing

old issue statement ## Intro Some proposals about representation of the `search.smartCase` in the "match case" button. The three proposals are - Ver. min modification - hide the "match case" button With least change, and it's a bit of a workaround. - Ver. original design - full implementation of "#42687" The original design was from #42687, and it's a good-enough way to express the current state of the search. - Ver. ternary states - new state for "match case" button Best way to express the current state of the search, but it's a bit complex. ## Background The `search.smartCase` has quite some history since [January 2018 (version 1.20)](https://code.visualstudio.com/updates/v1_20), yet recently (with it turned on), I've encountered the issue many times that it was working too quietly that I don't remember it's turned on. There are many issues regarding the same reason, like - #190022 - #187960 - #174452 ## Request I see the design was from #42687. I might have understood it wrong, but currently it seems not fully implemented. In the original image there were 4 status ![original-design](https://user-images.githubusercontent.com/323878/35653008-f89c2dee-069a-11e8-9ce3-8207e69cd09e.png) **state A**: searching with all lowercase with "match case" off **state B**: searching with uppercase, with "match case" off -> `search.smartCase` is activated, which turns on "match case" in a non-sticky way. **state C**: searching with uppercase, from **state B**, a click turns off "match case". By my assumption, this remembers "smart case" is off for this search, to distinguish from **state A**. **state D**: searching with uppercase, from **state C**, a click turns on "match case", in a sticky way. In my case (Version: 1.87.0-insider, Commit: e73419bef379f7956f024557cbf40bd3755a0645), by state order - I've never seen the state B, and the in somehow with `search.smartCase`. This is tolerable, or maybe the design was changed, because it might increase the UI complexity for user (I myself found it hard to understand the logic of the state C). - The unexpected part (might be a bug), with `search.smartCase` on, state C can never be visited. - Meanly, with uppercase users can only do case-sensitive search no matter how they interact with the "match case" button. Here the responsiveness of the UI element and the effect is broken, and I'm trying to fix it. ## Proposals ### Ver. min modification - hide the "match case" button The current effect of the `search.smartCase` is: 1. with full lowercase input, ignore cases 2. with full lowercase input, search only lowercase 3. with any uppercase input (including mixed input), do case sensitive search only. The "match case" button is irrelevant. This proposal is to hide the "match case" button in case 3. Since `search.smartCase` was for advanced users, they should know why the "match case" button disappeared: they have `search.smartCase` turned on. The improvement is to remind users that "save the trouble from clicking that button" ### Ver. original design - full implementation of "#42687" IMHO, with the expressive GUI, showing the state of `search.smartCase` is an improvement. I was a bit confused on the logic in "state C", specifically on what would happen on user's new input vs new search. b But writing out my assumption above helped me understand it. This can be somehow different than the current effect on `search.smartCase`, so there might be more work. ### Ver. ternary states - new state for "match case" button Thinking about **state C** in the original design > (searching with uppercase, from **state B**, a click turns off "match case". By my assumption, this remembers "smart case" is off for this search, to distinguish from **state A**) I see that there is a hidden "smart case" switch behind. I think the best way to express this is to show the "match case" button in a binary/ternary state: "on", "off", and, "smart case" if the `search.smartCase` is on. This is a bit more complex than the original design, but it's a good way to express the current state of the search. And further more, we can use the hollow / solid sparkling stars to express the "smart case" state. In addition, the "sticky" state of "match case" can be more clear for both user and developer of VSCode. ![smart-match-case](https://github.com/microsoft/vscode/assets/36828324/49173c9a-dfdd-4110-8437-fde83184f5ee) (Designing the icon is hard for me, this is just the idea) - hollow(left): "match case" in "smart case" mode, but all search text is lowercase - solid(right): "match case" in "smart case" mode, and there are uppercase letters in the search text ### further discussion and PR @roblourens, @octref if you are still working on related topics, I would be honored to discuss more with you about how `ripgrep` works, and about the best approach to show the state of case ignore in the search. I added a flowchart to help understand the logic of the "smart case" and "match case" button. I hope it helps. image Also, I'm open to make a PR for either the "Ver. min modification" or "Ver. ternary states" proposals.
ShinobiWPS commented 8 months ago

I'd like to add I had lost time because the UI haven't made clear i was using the Smart Case, i can easily imagine it happened to many more already.

andreamah commented 8 months ago

I just want to make sure I understand this correctly: You would like a better UI for when smart case is enabled, since there currently isn't anything in the UI that indicates that match case has been enabled automatically be smart case? I agree that we should have a better indication of when smart case has activated behind the scenes.

After reading your proposal, I have a few comments:

@daviddossett I was wondering if you had any input on this. As a bit of background, we have this setting search.smartCase, which, when disabled, will try to assume whether the user wants case-sensitivity or not. When it enables case-sensitivity for the user, there's no visual indication. Since the user can still use the case-sensitive button to force case sensitivity, we can't use the on/off of that to express whether it has auto-enabled or auto-disabled.

PabloLION commented 8 months ago
  • Regarding your proposal of Minimal Modification: Concealing the "Match Case" Button, I don't think that this would be more straightforward. If anything, we could disable the button when smartCase has automatically enabled case sensitivity. However, I think that people would question "why can't I change this?" if they've accidentally enabled the setting.

That's correct, and I didn't think about it. Maybe we can use a disable style with tooltips "managed by smart case".

  • We probably want to stay away from the sparkle icon, since we usually use that for AI generation. We currently don't have precedence of changing the icon in the input bar based on state IIRC, but perhaps this could be a way forward.

Yes again. It can be confusing. I thought it stands for "smart XXX". Anyway, the design is up to you guys. I can only help with the coding part, say if I'm going to make a PR after a final decision.

daviddossett commented 8 months ago

If anything, we could disable the button when smartCase has automatically enabled case sensitivity

If this is an explicit setting I as a user had to go find and toggle, it seems like it wouldn't be so bad to disable it assuming it uses a good tooltip w/ reasoning.

andreamah commented 8 months ago

Hmm, I'm not sure that we change the tooltip based on the setting. I think that the case that people are running into is:

daviddossett commented 8 months ago

Tooltip aside, I think disabling it would at minimum convey that the typical behavior has changed.

Image

I'd be surprised if we couldn't update a tooltip by listening to settings changes. We could alternatively change the icon after disabling the button itself but its always hard to convey this sort of state with an icon alone.

Another idea could be to add something to this text once a search has run:

Image

PabloLION commented 8 months ago
  • search something with a capital letter; the search viewlet looks as it always does, with match case seemingly off (although smart case is enabling match case behind the scenes).

Exactly, this to me is the root cause of the problem.

PabloLION commented 8 months ago

Another idea could be to add something to this text once a search has run:

Image

This seems to be a solution better than the Concealing the "Match Case" Button. It gives feedback while quire even less change to the current system.

The third proposal of "Ternary State" is trying to reduce the underlying difficulty of understanding the logic: The four states of the original design depends on two factors

  1. the sticky state from the last search
  2. the user's toggle of the current search.

And the fact that user's toggle sometimes affects the next search has made the comprehension even harder for me. Occasionally, I lose track if the case sensitive would be "on" or "off" for the next toggle and switching to "Ternary State" could resolve this. (Tho it needs much more work than adding some text to the search result summary)

andreamah commented 8 months ago

Tooltip aside, I think disabling it would at minimum convey that the typical behavior has changed.

The problem with this is that people can still use this button when smartCase is enabled. This would simply force the behavior one way.

I'd be surprised if we couldn't update a tooltip by listening to settings changes.

We could probably do this just fine, although I'm not sure if it solves the confusion. I'm not sure how many people wait to read the tooltip when they press the button.

PabloLION commented 8 months ago

I'm not sure how many people wait to read the tooltip when they press the button.

Tooltip was to explain the new icon, but I'm liking the solution with search result text. Should I update the issue statement? @andreamah For the users who notice the search result text, it would probably save the search or even creating an issue in this repo.

andreamah commented 7 months ago

Tooltip was to explain the new icon, but I'm liking the solution with search result text.

Could you refresh me on which solution this was? If it's the best one, it could be a good idea to edit the original comment to show the new direction we decide to go with this.

PabloLION commented 7 months ago

I think adding a text "smart-case search is active / inactive" above "80 results", or right after the search text-area would be a quick solution from David's comment.

Image

It's more logically related to the user input, so I prefer to show it closer to the input.


But if we are not in a hurry to ship this update, I would like the final product to have a ternary state on the "toggle case" button, because this month I used the search functionality more and I found that we need to open the setting to toggle the smart case was quite inconvenient. And we can implement this without adding the tooltips, since the button is self-explanatory. Also, this change can be applied to the "Find" command (id:actions.find).

PabloLION commented 7 months ago

I think adding a text "smart-case search is active / inactive" above "80 results", or right after the search text-area would be a quick solution from David's comment.

Image

It's logically more related to the user input, so I prefer to show it closer to the input.


But if we are not in a hurry to ship this update, I would like the final product to have a ternary state on the "toggle case" button, because this month I used the search functionality more and I found that we need to open the setting to toggle the smart case was quite inconvenient. And we can implement this without adding the tooltips, since the button is self-explanatory. Also, this change can be applied to the "Find" command (id:actions.find).

andreamah commented 7 months ago

We could add something like this first:

I think adding a text "smart-case search is active / inactive" above "80 results", or right after the search text-area would be a quick solution from David's comment.

Then think about something like a ternary state. That would be a major design change to how we do input toggles, so I would expect that it would take a lot of time to ensure that the team actually wants something like that (and that having something like that would be accessible to everyone).

PabloLION commented 5 months ago

Sorry that my GitHub inbox was full and I forgot about this issue, until I used this function again today 😅

May I try to make a PR for the quick solution? And meanwhile, we can still discuss on the ternary state.

andreamah commented 5 months ago

Sure! You can add a message or tooltip when smart case is enabled.