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

Reader: native browser 'find' function is not great #1180

Closed
johngodley opened this issue Dec 2, 2015 · 4 comments
Closed

Reader: native browser 'find' function is not great #1180

johngodley opened this issue Dec 2, 2015 · 4 comments
Labels
[Feature] Reader The reader site on Calypso. [Pri] Low Address when resources are available. [Status] Stale [Type] Enhancement

Comments

@johngodley
Copy link
Member

If you use the browsers 'find' function it can get very confused because of the overlayed nature of Calypso pages. This is because the find searches through all HTML, even if it's not visible. If you are looking at a single post in the Reader then the Reader itself is still in the HTML, and the find function searches there even though it's not visible to the user.

Example:

I am in the Reader and search for 'and' - it finds 21 occurrences on the page. Repeatedly pressing the find shortcut cycles through these with an orange highlight. Everything is working as I would expect.

in_browser

If I click on the post and then search for 'and' again I get the same number of results - 21 - even though only 3 are visible.

in_browser_after

Repeatedly pressing the find shortcut does nothing other than change the 1 of 21 number in the find box. This is because it is highlighting search terms in the main Reader, which is not visible.

Overall this isn't a major problem - the terms are still highlighted - and it's going to be pretty difficult to fix.

It causes more of a problem in the desktop app as there is no built-in find function and we need to use window.find. This does not highlight all the terms, but highlights individual terms each time it is called. In the case of the single post it just doesn't work well.

Note: I've only tried this in Chrome, which is what the desktop app is using.

@johngodley johngodley added [Type] Enhancement [Pri] Low Address when resources are available. labels Dec 2, 2015
@lancewillett lancewillett added the [Feature] Reader The reader site on Calypso. label Dec 4, 2015
@blowery
Copy link
Contributor

blowery commented Dec 4, 2015

Hmmm... What happens if you make the underlying stream visibility:hidden while the overlay is in place?

@johngodley
Copy link
Member Author

What happens if you make the underlying stream visibility:hidden while the overlay is in place?

That's a great suggestion! And it does the trick too. In Chrome the number of occurrences reflects what is just on screen, and in Electron it steps through the ones on screen and then wraps round.

In terms of the Reader, are there any downsides to setting the visibility to hidden? It's a small change but I'm not sure of the greater impact it may have.

@blowery
Copy link
Contributor

blowery commented Jan 12, 2016

I don't think it has any downsides, but we'll have to see. Might affect the way we scroll the underlying list when moving to the next article.

@lancewillett lancewillett changed the title Browser 'find' is not great Reader: native browser 'find' function is not great May 11, 2016
@stale
Copy link

stale bot commented Jan 11, 2018

This issue has been marked as stale because it hasn't been updated in a while. It will be closed in a week. If you would like it to remain open, can you please comment below and see what you can do to get things moving with this issue? Thanks! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Reader The reader site on Calypso. [Pri] Low Address when resources are available. [Status] Stale [Type] Enhancement
Projects
None yet
Development

No branches or pull requests

3 participants