Search - add search box control and implement search experience by KaiyuWang16 · Pull Request #3590 · microsoft/terminal

KaiyuWang16 · 2019-11-15T05:23:47Z

This is the PR for feature Search: #605
This PR includes the newly introduced SearchBoxControl in TermControl dir, which is the search bar for the search experience. And the codes that enable Search in Windows Terminal.

The PR that migrates the Conhost search module: #3279
Spec (still actively updating): #3299

PR Checklist

Closes Feature Request: Search #605
CLA signed. If not, go over here and sign the CLA
Tests added/passed
Requires documentation to be updated
I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

These functionalities are included in the search experience.

Search in Terminal text buffer.
Automatic wrap-around.
Search up or down switch by clicking different buttons.
Search case sensitively/insensitively by clicking a button. S. Move the search box to the top/bottom by clicking a button.
Close by clicking 'X'.
Open search by ctrl + F.

When the searchbox is open, the user could still interact with the terminal by clicking the terminal input area.

While I already have the search functionalities, currently there are still some known to-do works and I will keep updating my PR:

Optimize the search box UI, this includes:
1) Theme adaptation. The search box background and font color
should change according to the theme,
2) Add background. Currently the elements in search box are all
transparent. However, we need a background.
3) Move button should be highlighted once clicked.
Accessibility: search process should be able to performed without mouse. Once the search box is focused, the user should be able to navigate between all interactive elements on the searchbox using keyboard.

To test:

checkout this branch.
Build the project.
Start Windows Terminal and press Ctrl+F
The search box should appear on the top right corner.

…n Search

…n-Add-SearchBoxControl-and-Search-Experience

src/cascadia/TerminalApp/AppKeyBindings.cpp

src/cascadia/TerminalControl/SearchBoxControl.cpp

src/cascadia/TerminalApp/AppActionHandlers.cpp

src/cascadia/TerminalCore/terminalrenderdata.cpp

src/cascadia/TerminalControl/SearchBoxControl.h

src/cascadia/TerminalControl/SearchBoxControl.cpp

src/cascadia/TerminalControl/SearchBoxControl.xaml

zadjii-msft

I've only taken a cursory glance at a couple files so far, but I'm just gonna go ahead and pre-emptively block this PR until the spec (#3299) is merged

I am updating the spec today.

src/cascadia/TerminalApp/AppKeyBindingsSerialization.cpp

src/cascadia/TerminalControl/SearchBoxControl.idl

zadjii-msft

Fundamentally, this works so that's good. But there's definitely a few problems that need resolving before this can get merged. For me, the two remaining blocking issues are:

the placeholder text color is wrong in dark mode
RenderData::GetTextBufferEndPosition should not be GetCursorPosition

The other things I'm concerned about, but not blocking over:

"Find..." needs to be localizable
Search event should have a delegate with more args
Escape should be handled in the control too, as opposed to bubbling
The whole _goForward=!_goForward;_Search();_goForward=!_goForward thing is weird

src/cascadia/TerminalControl/SearchBoxControl.cpp

src/cascadia/TerminalControl/SearchBoxControl.xaml

src/cascadia/TerminalControl/TermControl.cpp

src/cascadia/TerminalControl/SearchBoxControl.idl

src/buffer/out/search.cpp

src/cascadia/TerminalControl/SearchBoxControl.cpp

DHowett-MSFT · 2019-12-16T22:22:45Z

There are still some outstanding comments.

KaiyuWang16 · 2019-12-16T23:48:51Z

There are still some outstanding comments.

Unblocking ones are moved to #3942

DHowett-MSFT

I'm comfortable with this as long as we follow up on the followup issues.

zadjii-msft

Thanks for cleaning those blocking things up! We can fix the rest in post 😝

zadjii-msft · 2019-12-17T15:52:21Z

(I'm merging this since there's a hyper-quorum on it)

ghost · 2019-12-17T15:52:31Z

Hello @zadjii-msft!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.

KaiyuWang16 · 2019-12-17T18:52:25Z

Thanks for cleaning those blocking things up! We can fix the rest in post 😝

Thanks for merging ! I am working on the follow-ups these days.

ghost · 2020-01-14T17:16:37Z

🎉Windows Terminal Preview v0.8.10091.0 has been released which incorporates this pull request.:tada:

Handy links:

kell18 · 2020-01-14T17:51:30Z

Yeees!

KaiyuWang16 added 18 commits October 21, 2019 16:10

Make search a shared component for conhost and terminal

66aeec0

Remove inclusion of deprecated interface file

02a280c

Code review changes, remove text buffer modification in Terminal

030961e

remove unreferenced objects to fix build errors

7eb30f6

Fix test failure, guarantee uiaData object is correctly initialized i…

fd0090e

…n Search

minor comment typo fix and format fix

c45ef9c

Add SearchBoxControl user control

eae5d88

do not add SearchBox to TermControl for now

1cdd414

First prototype of Search Experience

14586d8

Add calculation to correctly locate searched text on the visible view

9007f87

Search Capabilities - Add search box and search method

f2189c2

Reduce search and wrap-up time

767ff10

Add key bindings and close handler

eb48d74

Enable searchBox alignment change

821bbef

Use stackpanel to replace grid

9b74089

Fix typos and format issues

478405e

Merge branch 'master' into dev/kawa/605-Search-Experice-Implementatio…

f441add

…n-Add-SearchBoxControl-and-Search-Experience

comment out unused parameters in event handlers

a161352

KaiyuWang16 changed the title ~~Dev/kawa/605 search experice implementation add search box control and search experience~~ Search - add search box control and implement search experience Nov 15, 2019

codendone suggested changes Nov 20, 2019

View reviewed changes

ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Nov 20, 2019

KaiyuWang16 requested a review from DHowett-MSFT November 20, 2019 04:30

ghost removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Nov 20, 2019

KaiyuWang16 removed the request for review from DHowett-MSFT November 20, 2019 04:34

KaiyuWang16 added 2 commits November 20, 2019 17:04

Accessibility - tab navigation

7ef6cc9

Code review changes

5d8dc4c

KaiyuWang16 requested review from DHowett-MSFT, carlos-zamora and zadjii-msft November 21, 2019 19:01

zadjii-msft requested changes Nov 21, 2019

View reviewed changes

src/cascadia/TerminalApp/AppKeyBindingsSerialization.cpp Outdated Show resolved Hide resolved

ghost added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Dec 11, 2019

DHowett-MSFT reviewed Dec 12, 2019

View reviewed changes

src/cascadia/TerminalControl/SearchBoxControl.idl Show resolved Hide resolved

KaiyuWang16 added 2 commits December 11, 2019 21:14

Code review changes

43f3d6b

fix format

8f36e97

KaiyuWang16 mentioned this pull request Dec 12, 2019

Epic: Search v2 #3920

Open

20 tasks

zadjii-msft requested changes Dec 12, 2019

View reviewed changes

ghost added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Dec 12, 2019

KaiyuWang16 added 2 commits December 12, 2019 15:46

fix GetTextBufferEndPosition and some CR feedbacks

8bf675a

Use fix color on TextBox placeholder color to avoid XAML bug

5b95b01

microsoft deleted a comment from zadjii-msft Dec 16, 2019

DHowett-MSFT approved these changes Dec 17, 2019

View reviewed changes

carlos-zamora approved these changes Dec 17, 2019

View reviewed changes

zadjii-msft approved these changes Dec 17, 2019

View reviewed changes

zadjii-msft added the AutoMerge Marked for automatic merge by the bot when requirements are met label Dec 17, 2019

ghost merged commit d7ae8e6 into master Dec 17, 2019

ghost deleted the dev/kawa/605-Search-Experice-Implementation-Add-SearchBoxControl-and-Search-Experience branch December 17, 2019 15:52

ghost mentioned this pull request Jan 14, 2020

Feature Request: Search #605

Closed

This was referenced Jan 14, 2020

"find" not recognized as command for keybinding in profiles.json settings #4218

Closed

Add find action to keybinding commands in Settings Profile Schema #4219

Merged

zadjii-msft mentioned this pull request Apr 15, 2020

search #5359

Closed

DHowett mentioned this pull request Sep 15, 2020

Searching for a single character is broken #7607

Closed

This pull request was closed.

Conversation

KaiyuWang16 commented Nov 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Checklist

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zadjii-msft left a comment • edited by KaiyuWang16 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

zadjii-msft left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DHowett-MSFT commented Dec 16, 2019

Uh oh!

KaiyuWang16 commented Dec 16, 2019

Uh oh!

DHowett-MSFT left a comment

Choose a reason for hiding this comment

Uh oh!

zadjii-msft left a comment

Choose a reason for hiding this comment

Uh oh!

zadjii-msft commented Dec 17, 2019

Uh oh!

ghost commented Dec 17, 2019

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

Uh oh!

KaiyuWang16 commented Dec 17, 2019

Uh oh!

ghost commented Jan 14, 2020

Uh oh!

kell18 commented Jan 14, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

KaiyuWang16 commented Nov 15, 2019 •

edited

Loading

zadjii-msft left a comment •

edited by KaiyuWang16

Loading

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.