search

lbryio / lbry-desktop

A browser and wallet for LBRY, the decentralized, user-controlled content marketplace.
https://lbry.tech
MIT License
3.56k stars 413 forks source link

Enable text search (Ctrl+F) in Desktop #5877

Open infinite-persistence opened 3 years ago

infinite-persistence commented 3 years ago

Often while searching, I want to go back to an entry way up in the search list that I still remember it's title partially. This is easy to do on the web browser via Find (or Ctrl+F), but not so on Desktop.

It would also be helpful for searching through long comments.

tzarebczan commented 3 years ago

This should be fixed by https://github.com/lbryio/lbry-desktop/pull/5874 right?

infinite-persistence commented 3 years ago

Differerent -- this one adds text search to electron.

tzarebczan commented 3 years ago

Ah roger!

mayeaux commented 3 years ago

I looked into this, and it's possible but possibly not very trivial to implement.

Possible to do find within a page? Cmd/Ctrl+F? | Electron Issue https://github.com/electron/electron/issues/2037

Electron does offer this API for finding content: https://www.electronjs.org/docs/api/web-contents#contentsfindinpagetext-options

And then an event listener that you could hook into as well: https://www.electronjs.org/docs/api/web-contents#event-found-in-page

So presumably you could build a small GUI that could interact with this API. In fact, someone has built a module for achieving this ctrl + f search effect, but it's not clear that it will work in later versions of Electron (I opened a ticket asking about it: https://github.com/TheoXiong/electron-find/issues/18)

https://github.com/TheoXiong/electron-find

So best case scenario, that module would work on Electron v12 because it seems to be coded quite well, and then you could just plug and play it. Worst case you'd have to build something by hand but it should be possible if not a little annoying (and at least you have this guy's working implementation to go off of).

I agree it's pretty annoying and jarring to not have CTRL + F search functionality, especially given that it's available on web browsers. Depends on dev time and perceived impact on users if this is considered worth prioritizing, but the good news is that Electron has an API that at least does make it possible.

wilkerlucio commented 3 years ago

I'm going through the process now, and I can tell it's painful.

I couldn't figure how to make electron-find work with Electron 12, pulling the polyfill for remote on Electron didn't do the trick.

As I'm trying to implement manually (using the findInPage helper), there are pain points, because it's a UI event that needs to go in the background to trigger something back again. I got that part working, but every time the find is fired, the input that's doing the search loses focus. I can make it refocus, but the UI feels kinda bad. Also, if you trigger findInPage multiple times it does cycle between the occurrences, but if I do the focus back thing, it loses track of it.

So I see electron-find and other solutions used to rely on <webview /> element to avoid this focus madness, but Electron warns to not use it. I didn't try yet, just wanna say it's a really painful process, would be great if Electron provides something easy to use out of the box. It is great to have the flexibility with findInPage, but I believe most people just want a replication of what the browsers do, and Electron could provide that too.

tzarebczan commented 3 years ago

I just realized this was not linked to the PR that was in process, sounds like the author is taking another shot: https://github.com/lbryio/lbry-desktop/pull/5938

TomasGonzalez commented 3 years ago

I'm going through the process now, and I can tell it's painful.

I couldn't figure how to make electron-find work with Electron 12, pulling the polyfill for remote on Electron didn't do the trick.

As I'm trying to implement manually (using the findInPage helper), there are pain points, because it's a UI event that needs to go in the background to trigger something back again. I got that part working, but every time the find is fired, the input that's doing the search loses focus. I can make it refocus, but the UI feels kinda bad. Also, if you trigger findInPage multiple times it does cycle between the occurrences, but if I do the focus back thing, it loses track of it.

So I see electron-find and other solutions used to rely on <webview /> element to avoid this focus madness, but Electron warns to not use it. I didn't try yet, just wanna say it's a really painful process, would be great if Electron provides something easy to use out of the box. It is great to have the flexibility with findInPage, but I believe most people just want a replication of what the browsers do, and Electron could provide that too.

I already have a PR opened #5938 I went around the focus thing by searching when enter is pressed, this doesn't match the browser behavior but at least makes searching possible. I'm gonna give it a try with the electron-find later, and possibly with the webview approach if I also have no success.

TomasGonzalez commented 3 years ago

I'm going through the process now, and I can tell it's painful.

I couldn't figure how to make electron-find work with Electron 12, pulling the polyfill for remote on Electron didn't do the trick.

As I'm trying to implement manually (using the findInPage helper), there are pain points, because it's a UI event that needs to go in the background to trigger something back again. I got that part working, but every time the find is fired, the input that's doing the search loses focus. I can make it refocus, but the UI feels kinda bad. Also, if you trigger findInPage multiple times it does cycle between the occurrences, but if I do the focus back thing, it loses track of it.

So I see electron-find and other solutions used to rely on <webview /> element to avoid this focus madness, but Electron warns to not use it. I didn't try yet, just wanna say it's a really painful process, would be great if Electron provides something easy to use out of the box. It is great to have the flexibility with findInPage, but I believe most people just want a replication of what the browsers do, and Electron could provide that too.

Update: webview method will keep unfocusing unless you also put the whole app into another separate webview, so I ditched this idea. Now I'm going to proceed and try to brute force search the DOM and compare the innerhtml of each element to the search input.