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

Extended API: how exactly do we list results? #7

Closed
domenic opened this issue Mar 9, 2018 · 4 comments
Closed

Extended API: how exactly do we list results? #7

domenic opened this issue Mar 9, 2018 · 4 comments

Comments

@domenic
Copy link
Contributor

domenic commented Mar 9, 2018

We list several APIs here:

  • setDeferredResults
  • setDeferredResultsBeforeFirst
  • setDeferredResultsAfterLast

Which of these are we proposing? Probably not all of them. Which ones do we need?

Keeping in mind the example code, I am thinking maybe we just need setDeferredResults (which we should rename "addResults"). If the idea is for the browser to re-do the find after we set the results, then we don't need to tell the browser if they're before the first or after the last.

@rakina rakina closed this as completed in 0cbe46d Mar 13, 2018
@rakina
Copy link
Owner

rakina commented Mar 13, 2018

Hmm, I just thought about this again. Should the browser really re-do the find after we set the results, for all the results? What if the result items are all far away from each other? That will mean loading a lot of things into the DOM.

If we have beforeFirst/afterLast, then the browser can re-do the find only when the browser needs it (at the start/end of browser's own result).

@rakina rakina reopened this Mar 13, 2018
@domenic
Copy link
Contributor Author

domenic commented Mar 14, 2018

I can see how giving the browser a hint on how much of the page to re-do the find on could save a bit of processing power. But I'm not sure whether that savings is worth asking authors to manually figure out whether their results are before or after the current search position.

But maybe I am confused. You said

What if the result items are all far away from each other? That will mean loading a lot of things into the DOM.

How would you propose avoiding loading a lot of things into the DOM in these cases?

@rakina
Copy link
Owner

rakina commented Mar 14, 2018

How would you propose avoiding loading a lot of things into the DOM in these cases?

My idea is that the browser should only call the callback in the added results when the active match is nearing those results (nearing the end or the start of browser's own results) one by one. But I guess it's not really worth it to make the authors figure out which function to call..

This is related to #10. If addResult makes the browser do the callbacks of the results and redo find immediately, I'm not really sure of the difference of using e.waitUntil(callback) and adding a FindInPageResult with the same callback.

@domenic
Copy link
Contributor Author

domenic commented Mar 14, 2018

My idea is that the browser should only call the callback in the added results when the active match is nearing those results (nearing the end or the start of browser's own results) one by one. But I guess it's not really worth it to make the authors figure out which function to call..

Oh, no, you're totally right, we want this. I was confused indeed. I was thinking we would only call the callback when they go to the particular FindInPageResult in question. But I see the problem: how does the browser know when the user wants to go to that FindInPageResult?

I think setDeferredResultsBeforeFirst and setDeferredResultsAfterLast is better than just addResults. (Maybe keep the simpler name like addResultsBeforeFirst.) Maybe there is something better, but we should probably go back to that split for now.

@rakina rakina closed this as completed in 0a8af64 Mar 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants