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

Common input box with filters #156179

Closed
rebornix opened this issue Jul 25, 2022 · 13 comments
Closed

Common input box with filters #156179

rebornix opened this issue Jul 25, 2022 · 13 comments
Assignees
Labels
debt Code quality issues ux User experience issues

Comments

@rebornix
Copy link
Member

rebornix commented Jul 25, 2022

With Find Widget introduced in File Explorer and Notebook Editor, we now have multiple implementations for input box with multiple filter/toggles. We reused FindInput which has 3 built-in filters (caseSensitivity, wholeWord and regex) and had to add additional filters in our own ways. File Explorer and Notebook handles this differently and there isn't a clear path of how they can be unified and how other input box users can adopt this in the future.

In addition, Problems panel and Terminal recent commands have their own input box with a single filter, which are not inherited from find input.

Variations

Text Editor

  • 3 built-in filters
  • Additional filter (Find in Selection) as a separate button

image

Terminal

  • 3 built-in filters only

image

Webview

  • Built-in filters all disabled/hidden

image

File Explorer

  • Built-in filters disabled/hidden
  • Additional filter shown as a toggle inside the input box

image

Notebook

  • 3 built-in filters only
  • Additional filters shown in a dropdown/context menu

image

Problems

  • Multiple filters which are specific to Problems only

image

Proposal

The input boxes from find widgets shown above all derive from the original FindWidget in Monaco, which only has 3 builtin filters and they are no longer universal as not all embedders have the capability of implementing all of them. Embedders also have the need to the extend the filters which is not well supported, leading to the fragmentation of the find widgets.

The proposal here is experimenting if we can introduce a generic input box with dynamic filters support at the base/platform level (not from editor or workbench). The common features are:

  • Support filters as Toggle button
  • Support filters as Dropdown/Context menu
  • Broadcast events when filter state changes

The UI for it can be a mix of the input boxes from File Explorer and Notebook, and embedders can contribute dynamic filters and declare how they should be rendered (as Toggle or Dropdown) statically.

A draft for the interfaces of the new input box and its filters:

interface InputFilter {
    value: boolean;
    icon: Codicon;
    isPrimary: boolean;
}

interface InputFilters {
    [key: string]: InputFilter;
}

type FilterUpdateEvent<T> = { [K in keyof T]?: string };

interface InputWithFilters<T = InputFilters> {
    filters: T;
    onDidChangeFilter: Event<FilterUpdateEvent<T>>;
}

The input box in the Notebook Find Widget can then be declared as

interface NotebookEditorFindInputFilters extends InputFilters {
    caseSensitive: InputFilter;
    wholeWord: InputFilter;
    regex: InputFilter;
    searchInMarkdown: InputFilter;
    searchInOutput: InputFilter;
}

type NotebookEditorFindInput = InputWithFilters<NotebookEditorFindInputFilters>;

// notebook editor find input filter object
{
    caseSensitive: { value: false, icon: Codicon.caseSensitive, isPrimary: true },
    wholeWord: { value: false, icon: Codicon.wholeWord, isPrimary: true },
    regex: { value: false, icon: Codicon.regex, isPrimary: true },
    searchInMarkdown: { value: false, icon: Codicon.markdown, isPrimary: false },
    searchInOutput: { value: false, icon: Codicon.output, isPrimary: false }
}

cc @joaomoreno @Tyriar @andreamah @mjbvz feedbacks welcome!

@rebornix rebornix assigned rebornix, joaomoreno and Tyriar and unassigned joaomoreno and Tyriar Jul 25, 2022
@rebornix rebornix added debt Code quality issues ux User experience issues labels Jul 25, 2022
@rebornix rebornix added this to the On Deck milestone Jul 25, 2022
@miguelsolorio
Copy link
Contributor

Love that we are tackling this problem! From a design perspective, I think we can establish some principles for how the interactions should work:

  • Actions with on/off states become "toggles"
  • Dropdown menu options that provide toggle options should use the generic "filter" icon
  • Dropdown menu options that change the input (like adding queries) become "actions" and don't require an on/off state

Things that are unclear so far:

  • Should dropdown "actions" use a different icon or use the same?
  • Should in-context find widgets appear differently than search/filter inputs?

@Tyriar
Copy link
Member

Tyriar commented Aug 12, 2022

Here are what each of the inputs called out above are built from, plus the quick pick which we also want filter support for.


Text editor

FindWidget                        editor/contrib/find/browser/findWidget.ts
  FindInput                       base/browser/ui/findinput/findInput.ts
    HistoryInputBox               base/browser/ui/inputbox/inputBox.ts
      InputBox                    base/browser/ui/inputbox/inputBox.ts

Terminal

TerminalFindWidget                workbench/contrib/terminal/browser/terminalFindWidget.ts
  SimpleFindWidget                workbench/contrib/codeEditor/browser/find/simpleFindWidget.ts
    FindInput                     base/browser/ui/findinput/findInput.ts
      HistoryInputBox             base/browser/ui/inputbox/inputBox.ts
        InputBox                  base/browser/ui/inputbox/inputBox.ts

Webview

WebviewFindWidget                 workbench/contrib/webview/browser/webviewFindWidget.ts
  SimpleFindWidget                workbench/contrib/codeEditor/browser/find/simpleFindWidget.ts
    FindInput                     base/browser/ui/findinput/findInput.ts
      HistoryInputBox             base/browser/ui/inputbox/inputBox.ts
        InputBox                  base/browser/ui/inputbox/inputBox.ts

Problems

ActionBar                         base/browser/ui/actionbar/actionbar.ts
  MarkersFilterActionViewItem     workbench/contrib/markers/browser/markersViewActions.ts
    ContextScopedHistoryInputBox  platform/history/browser/contextScopedHistoryWidget.ts
      HistoryInputBox             base/browser/ui/inputbox/inputBox.ts
        InputBox                  base/browser/ui/inputbox/inputBox.ts

Notebook

NotebookFindWidget                workbench/contrib/notebook/browser/contrib/find/notebookFindWidget.ts
  SimpleFindReplaceWidget         workbench/contrib/notebook/browser/contrib/find/notebookFindReplaceWidget.ts
    NotebookFindInput             workbench/contrib/notebook/browser/contrib/find/notebookFindReplaceWidget.ts
      FindInput                   base/browser/ui/findinput/findInput.ts
        HistoryInputBox           base/browser/ui/inputbox/inputBox.ts
          InputBox                base/browser/ui/inputbox/inputBox.ts

File explorer

FindController                    base/browser/ui/tree/abstractTree.ts
  FindWidget                      base/browser/ui/tree/abstractTree.ts
    FindInput                     base/browser/ui/findinput/findInput.ts
      HistoryInputBox             base/browser/ui/inputbox/inputBox.ts
        InputBox                  base/browser/ui/inputbox/inputBox.ts

Quick pick

QuickInputBox                     base/parts/quickinput/browser/quickInputBox.ts
  InputBox                        base/browser/ui/inputbox/inputBox.ts

@Tyriar
Copy link
Member

Tyriar commented Aug 12, 2022

Toggles

Text editor, terminal, webview, notebook

  • FindInput uses Toggle (base/browser/ui/toggle/toggle.ts) to implement the toggles.

Filter toggle

File explorer implements the filter as a toggle as above

  • FindWidget (base/browser/ui/tree/abstractTree.ts) uses ModeToggle which extends Toggle (base/browser/ui/toggle/toggle.ts)

Filter dropdown

Notebook and problems uses an ActionBar to implement the filters dropdown

Notebook:

  • NotebookFindFilterActionViewItem extends DropdownMenuActionViewItem

Problems:

  • FiltersDropdownMenuActionViewItem extends DropdownMenuActionViewItem

@Tyriar
Copy link
Member

Tyriar commented Aug 12, 2022

After looking into how everything works, here are my proposed changes. Let me know what everyone thinks.

Should in-context find widgets appear differently than search/filter inputs?

@misolori I don't see any reason to make them look differently? Do you have an example of why this would be good?

@miguelsolorio
Copy link
Contributor

I like those proposals

@misolori I don't see any reason to make them look differently? Do you have an example of why this would be good?

The reason why I bring this up is that some find widgets have resize handles and some don't while others have a grabber to move it around. Not sure if this is a big deal but what came to mind when writing that.

@rebornix
Copy link
Member Author

rebornix commented Aug 12, 2022

@Tyriar I like where we are heading towards, here are some of my thoughts for the proposal above

Move _showOptionButtons from FindInput.ctor into IFindInputOptions and rename it to talk about toggles, not buttons. I'm not sure why it wasn't there to begin with?

_showOptionButtons is a bad name, it should be called *toggles.

I think we should keep the toggle (mode) and filter concepts separate to make their intention more clear

👍 for separating them, and I'm in favor of filterActionViewItemProvider?: IActionViewItemProvider as it's hard to define an universal model for additional filters. With IActionViewItemProvider it's fully controlled by embedders and we can have keybinding support for the actions.

I'm not sure about if we are calling them Toggle/Filter, as Toggle is referring to the UI/UX, while Filter is referring to the functionality. caseSensitivity/matchingWord/regex are about how we match queries (SearchOptions) and additional filters are about where we search (SearchScopes).

The text editor find in selection either moves into a filter icon dropdown or an additional toggle. I think this is out of the scope of this issue and should be handled by text editor owners

We can bring this to UX sync. I think in the long run we want to make it Filter/dropdown then we can introduce semantic search scopes (search in comments/strings/code). cc @alexdima @hediet .

@Tyriar
Copy link
Member

Tyriar commented Aug 12, 2022

I'm not sure about if we are calling them Toggle/Filter, as Toggle is referring to the UI/UX, while Filter is referring to the functionality.

How about Toggle and FilterToggle then as they are toggles but will always live in a dropdown called "Filters"?

caseSensitivity/matchingWord/regex are about how we match queries (SearchOptions) and additional filters are about where we search (SearchScopes).

I'm not sure we're currently that strict wrt options vs scopes. For the terminal anyway we're not sure whether we're going to have a toggle or a filter yet and the decision is currently coming down to are we able to get an icon to represent fuzzy search. I see all toggles and filters are just search options and/or modes, the main differentiator for me is whether they're important enough to be in the list vs in the filters dropdown.

With that said, maybe we should be strict about this and make "Toggles" = options/modes/how something is searched, and "Filter Toggles" = scope/where/what is searched? @misolori thoughts?

@rebornix
Copy link
Member Author

the main differentiator for me is whether they're important enough to be in the list vs in the filters dropdown

This makes good sense. If we are not trying to differentiate the functionalities of options in the list and the ones in the dropdown, then their difference is UI (where they get rendered), which is similar to Toolbar/ActionBar's primary/secondary grouping.

@miguelsolorio
Copy link
Contributor

I see all toggles and filters are just search options and/or modes, the main differentiator for me is whether they're important enough to be in the list vs in the filters dropdown.

To the user, it doesn't make much difference if an option is part of the input or in a dropdown, they are just "options", so I agree with this. I think in order to move into a dropdown there needs be more than one option.

@Tyriar
Copy link
Member

Tyriar commented Aug 17, 2022

UX sync feedback:

  • No pushback on find in selection being in the filters dropdown, though that point might have been buried under all the other things I was talking about. @rebornix we should do a ux channel poll to gauge interest if you want to follow that up
  • Consensus seemed to be 👍 to strict division of search modes vs filters dropdown

@rebornix
Copy link
Member Author

No pushback on find in selection being in the filters dropdown, though that point might have been buried under all the other things I was talking about. @rebornix we should do a ux channel poll to gauge interest if you want to follow that up

Let's start one to ensure others who use find in selection in the team and Editor folks are aware of the coming UX changes.

@Tyriar
Copy link
Member

Tyriar commented Aug 19, 2022

@rebornix I'm a little surprised how positive the reception was, makes a lot more sense with your find in comments/string/brackets suggestion though. I forked that off to #158598 for you to tackle in a debt week

Tyriar added a commit that referenced this issue Aug 19, 2022
This makes it distinct from the common dropdown icon.

Part of #156179
@Tyriar
Copy link
Member

Tyriar commented Aug 30, 2022

Move the filters dropdown as implemented by DropdownMenuActionViewItem to be managed inside FindInput

I tried this and had a bit of trouble making it nice. I suggest revisiting making it generic when we next look at adopting the dropdown in another FindInput.

@Tyriar Tyriar closed this as completed Aug 30, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Oct 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debt Code quality issues ux User experience issues
Projects
None yet
Development

No branches or pull requests

4 participants