1941575 - Add search toolbar in the Raw Response footer similar to the Debugger one

Status: ASSIGNED

(DevTools :: Netmonitor, enhancement, P3)

Description

[Nicolas Chevobbe [:nchevobbe]]

The search UI from the CodeMirror (5) instance doesn't have navigation button, and hitting Enter makes it disappear.
Results can be cycled with Cmd+G and Cmd+Shift+G, but this is pretty undiscoverable.
Would be nice if we had an obvious way to do this in the UI with navigation button, and maybe add extra search params like regex, case sensitivity and so on

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

Note that Bug 1946793 should make the current situation a bit better (the UI won't close, and [Shift+]Enter will cycle through results)

Comment 2

[Chris Vander Linden]

This looks like a fun one! I'll take it, if it's still available.

Comment 3

[Nicolas Chevobbe [:nchevobbe]]

(In reply to Chris Vander Linden from comment #2)

This looks like a fun one! I'll take it, if it's still available.

Thanks Chris, it's now yours :)
Note that bomsy is currently migrating the Netmonitor to CodeMirror 6 (https://phabricator.services.mozilla.com/D259089), so you might have some (hopefully not too much) conflicts

Comment 4

[Hubert Boma Manilla (:bomsy)]

Thanks Chris. for taking a look at this.

Just to add a little bit.
Our goal is to use same Search UI as that enabled in the debugger panel source editor. It can be displayed using Ctrl+F in the source editor.
So the direction with this, would be to see if we can make the SearchinFileBar component more generic so it can be reused with the Source Preview in the netmonitor.

Comment 5

[Chris Vander Linden]

This sounds great! I plan to spend the next few days walking through the code to make sure I fully understand it. I just have a few questions.

Is the goal to refactor the current debugger SearchinFileBar so it can also be used with the network raw response panel or should I just use that UI for the time being?

I'm about to start up my fall semester for computer science. How time sensitive is the feature?

I'm still relatively new to working on a development team. What is the best way to communicate if I have questions?

Comment 6

[Nicolas Chevobbe [:nchevobbe]]

(In reply to Chris Vander Linden from comment #5)

This sounds great! I plan to spend the next few days walking through the code to make sure I fully understand it. I just have a few questions.

Is the goal to refactor the current debugger SearchinFileBar so it can also be used with the network raw response panel or should I just use that UI for the time being?

Ideally, it would be nice to have a first patch that moves https://searchfox.org/mozilla-central/source/devtools/client/debugger/src/components/Editor/SearchInFileBar.js (and its associated CSS file) to https://searchfox.org/mozilla-central/source/devtools/client/shared/components/
Then we can use it from the netmonitor SourcePreview component

I'm about to start up my fall semester for computer science. How time sensitive is the feature?

There's no rush at all. We do have a working search, so there's no deadline at all for that. We're already glad you're helping us :)

I'm still relatively new to working on a development team. What is the best way to communicate if I have questions?

You can ask question here, or if you have some questions about some code you wrote, you can push your patch to Phabricator as a WIP and ask questions there. Finally, you might ask question in https://chat.mozilla.org/#/room/#devtools:mozilla.org (note that most of the team is in Europe)

Comment 7

[Chris Vander Linden]

I’m jumping back into Bug 1941575 now that I’m between semesters and wanted to share a simple phased plan to make sure I understand how to approach this.

Phase 1 - Decouple SearchInput from Debugger Redux:
• Today SearchInput is Redux-connected (reads getSearchOptions and dispatches setSearchOptions).
• I’ll refactor it so SearchInput becomes a “dumb” UI: it receives searchOptions (and a setter/callback) via props instead of importing selectors/actions directly.
• SearchInFileBar (Debugger side) will supply those props (so all Debugger selectors/actions stay in the container/wrapper layer, not inside the shared UI component).

Phase 2 - Move SearchInput to shared:
Once it’s unconnected, move SearchInput to devtools/client/shared/components/ (or the location you prefer), and keep Debugger working by updating imports (or leaving a thin shim at the old path if needed to avoid large churn).

Phase 3 - Decouple and share SearchInFileBar:
Next, apply the same pattern to SearchInFileBar: remove Debugger-specific selectors/actions from the component itself and have those injected via props.

My question is where should the Redux wiring live? Should there be a Debugger-only wrapper (connected) that renders a shared/dumb SearchInFileBar? That keeps the large Debugger Editor component clean (it would keep rendering a “SearchInFileBar” component and pass editor, but the wrapper would provide the Debugger-specific props). Or, should the props be passed in the Debugger Editor component?

Thanks!

Chris

Comment 8

[Chris Vander Linden]

Attachment: Bug 1941575 - Move SearchInFileBar to shared and add Debugger wrapper. r=bomsy

Comment 9

[Hubert Boma Manilla (:bomsy)]

Tah(In reply to Chris Vander Linden from comment #7)

I’m jumping back into Bug 1941575 now that I’m between semesters and wanted to share a simple phased plan to make sure I understand how to approach this.

Phase 1 - Decouple SearchInput from Debugger Redux:
• Today SearchInput is Redux-connected (reads getSearchOptions and dispatches setSearchOptions).
• I’ll refactor it so SearchInput becomes a “dumb” UI: it receives searchOptions (and a setter/callback) via props instead of importing selectors/actions directly.
• SearchInFileBar (Debugger side) will supply those props (so all Debugger selectors/actions stay in the container/wrapper layer, not inside the shared UI component).

Phase 2 - Move SearchInput to shared:
Once it’s unconnected, move SearchInput to devtools/client/shared/components/ (or the location you prefer), and keep Debugger working by updating imports (or leaving a thin shim at the old path if needed to avoid large churn).

Phase 3 - Decouple and share SearchInFileBar:
Next, apply the same pattern to SearchInFileBar: remove Debugger-specific selectors/actions from the component itself and have those injected via props.

My question is where should the Redux wiring live? Should there be a Debugger-only wrapper (connected) that renders a shared/dumb SearchInFileBar? That keeps the large Debugger Editor component clean (it would keep rendering a “SearchInFileBar” component and pass editor, but the wrapper would provide the Debugger-specific props). Or, should the props be passed in the Debugger Editor component?

Yeah that sounds like a good plan.

Thanks for your breakdown here, sounds like a good plan. This particular bug is for implementing the shared Search Component in the netmonitor.

I've filed other bugs for moving the SearchInput Component (Bug 2019256) and the SearchInFileBar Component (Bug 2019260) from the debugger to the shared folder.

Comment 10

[Phabricator Automation]

Comment on attachment 9537100 [details]
Bug 1941575 - Move SearchInFileBar to shared and add Debugger wrapper. r=bomsy

Revision D278736 was moved to bug 2019256. Setting attachment 9537100 [details] to obsolete.