[search-in-workspace] Add history to input fields

[gbodeen]

Signed-off-by: Gabriel Bodeen [email protected]

What it does

Fixes #8959.

Some debatable choices:

• VS Code's way of handling histories on input fields involves a pretty significant class hierarchy and detailed API. I don't believe that level of refactoring is justified here yet.
• SIW uses uncontrolled input fields and direct manipulation of the element value, so this component cooperates with that in a way that is somewhat unidiomatic React. As a result this component may not be easily reusable elsewhere. The alternative would be a significant refactor.
• Setting the history is debounced with 1 full second delay on the theory that we should only keep entries when the user waited long enough to look down at the list of results.

Mostly these choices were driven by KISS.

How to test

1. Use the search-in-workspace widget. Search for several different things. All previous behavior should still work, except as regards the up and down arrow keys.
2. The up key now switches the text value of the input field to the previous value, if any. The down key now switches the text value to the next value, if any. The results tree should update. This should work for both the search input and the replace input.

Review checklist

• as an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

• as a reviewer, I agree to behave in accordance with the review guidelines

[colin-grant-work]

In VSCode, the history is also stored across sessions, so it would be nice to add the history to the state of SIW widget, and restore it on restart:

theia/packages/search-in-workspace/src/browser/search-in-workspace-widget.tsx

Lines 164 to 176 in 8eb33e3

	storeState(): object {
	return {
	matchCaseState: this.matchCaseState,
	wholeWordState: this.wholeWordState,
	regExpState: this.regExpState,
	includeIgnoredState: this.includeIgnoredState,
	showSearchDetails: this.showSearchDetails,
	searchInWorkspaceOptions: this.searchInWorkspaceOptions,
	searchTerm: this.searchTerm,
	replaceTerm: this.replaceTerm,
	showReplaceField: this.showReplaceField
	};
	}

[gbodeen]

Hm, github is really unhappy with the rebase.

[vince-fugnitto]

I noticed a bug when restoring the input history after reload:

• perform multiple searches (ex: 3-4)
• delete the content in the input (so it is empty)
• reload the application
• attempt to cycle through the input history (arrow up or arrow down)
• the history is unavailable

It looks like history is only available if the app is reloaded when there is content in the input field.

[gbodeen]

It looks like history is only available if the app is reloaded when there is content in the input field.

Thanks for catching that. It's now fixed.

[colin-grant-work]

Thanks for catching that. It's now fixed.

There's still something slightly funny going on here. I did the following:

1. Enter some values
2. Navigate in the history a bit
3. Come back to the end of the history
4. Clear the input.
5. Refresh

After the refersh, the state was restored, but when I pressed ↑ it took me to the penultimate item in my history, and ↓ then took me to the last item in the history. It seems that it kept track of the history, but believed that the index was targeting the last non-empty item, even though that wasn't what was displayed. Maybe the empty string should be added to the history for the purposes of index tracking, but ignored if the user is trying to navigate the history?

[colin-grant-work]

These could all be this.someRef.current?.setState(...)

[gbodeen]

There's still something slightly funny going on here. I did the following ... ↑ it took me to the penultimate item in my history, and ↓ then took me to the last item in the history.

This is actually correct behavior! VS Code does the same. In both, what's happening is that the index of your location in the history is persisted until changed by ↑ or ↓ or adding a new non-empty value . The behavior is easiest to observe if you follow these steps:

1. Search for 1, 2, 3, 4, 5 in order and give each a moment to exceed the debounce delay.
2. Hit ↑↑ to get back to the 3. Clear the input and refresh.
3. You should now see a still-blank input as the searchTerm saved in the widget state is the empty string. However, the index of your position in the history is similarly still where you left it (i.e. 2, for history[2]='3', even though the 3 is not currently displayed). So if you hit ↑ you will then see 2, or if you hit ↓ you will see 4.

[colin-grant-work]

This is actually correct behavior! VSCode does the same.

I'm not sure that the latter fact implies the former, in this case 😉. At least, 'Show me the item before the item I'm not seeing,' isn't what I thought ↑ meant.

[gbodeen]

At least, 'Show me the item before the item I'm not seeing,' isn't what I thought ↑ meant.

That's quite reasonable. After some discussion outside GitHub, we have two proposed differences in behavior from VS Code:

• If you clear an entry, then either ↑ or ↓ will first restore that entry, then navigate the history from that point.
• If you are at the end of the history and you press ↓, you get a blank field ready for typing.

IMO, they're easier to understand and more convenient.

[colin-grant-work]

We'll merge this one next week, if there are no objections.

[gbodeen]

(Squashed & rebased onto the tip of master as requested. Rebuilt & confirmed it still behaves properly.)