search

mozilla / pdf.js

PDF Reader in JavaScript
https://mozilla.github.io/pdf.js/
Apache License 2.0
48.6k stars 9.99k forks source link

Search/Find is not visible in Mobile Devices #6191

Closed gsampath127 closed 7 years ago

gsampath127 commented 9 years ago

Can anyone give explanation why PDF Js lacks features like search in mobile devices

timvandermeij commented 9 years ago

I think it is there, but the search button disappears on small widths (try with https://mozilla.github.io/pdf.js/web/viewer.html and resizing the browser). The current viewer has not been made with mobile devices in mind and we are hoping to make a separate mobile viewer, so I'm marking this as a feature for now.

Thunderforge commented 9 years ago

I did a bit of investigation and found that this is is due to viewer.html line 214 where div with id findbar has the hiddenSmallView class. Removing this fixes the issue of find disappearing when you use browser-level zoom (the linked issue), which would mean those with visual impairments could still use the find feature. I presume it would also make it usable on mobile devices.

Would this be an acceptable change to make in a pull request?

timvandermeij commented 9 years ago

Well, I assume that it is not there for nothing, so what happens when you remove that and make the screen smaller? I think there is simply not enough room in the toolbar for that button too, and buttons will overlap.

Thunderforge commented 9 years ago

If you remove the class, the button remains and seems to work for about all usable resolutions. The whole window looks good as small as 550 px (at 100% browser zoom).

image

If you go any smaller, the "Page" label and the zoom field disappear, which also happens when you shrink down to a certain size (about 525 px). There is also a strange overlap thing with the zoom out button, which on further testing I found can actually happen independently of this change. I'll file an issue for that shortly.

image

So by hiding the find bar, we save about 25 px. I'm hoping that that will be an insignificant amount. Perhaps we could hide the next and previous buttons instead of the find button, since you can still get to the next page via the page number field and through scrolling?

timvandermeij commented 9 years ago

I actually think that is is more an overall responsiveness question: which buttons do we really want to show and which can we hide? As you have seen in the screenshots above, there are currently some issues with the responsiveness, i.e., the zoom button overlap. I tihnk the overall toolbar responsiveness needs to be reconsidered a bit.

timvandermeij commented 9 years ago

By removing that class, the zoom buttons overlap much more for me than with that class. With that class it only happens at around 790px width.

Thunderforge commented 9 years ago

In that case, what is the best way to move forward on this? Since this issue affects accessibility (using browser-level zoom), it is an important issue for us.

Snuffleupagus commented 9 years ago

I actually think that is is more an overall responsiveness question: which buttons do we really want to show and which can we hide?

That is probably an question that requires input from the UX team, and discussions with the main developers.

I think the overall toolbar responsiveness needs to be reconsidered a bit.

If so, that should probably not be done piecewise, since there's a risk that we'd end up in a weird intermediate state before it's done. (Which would be bad, since we ship the default viewer in Firefox.)

By removing that class, the zoom buttons overlap much more for me than with that class. With that class it only happens at around 790px width.

I can confirm this, that does not look particularity good!

timvandermeij commented 9 years ago

We need to define from a UX point of view which buttons should always remain on the toolbar and which we can hide or put in the secondary toolbar (the >> button), i.e., provide arguments why one button is more important than another on smaller displays. The UX team can help here (contact them on #ux on irc.mozilla.org, for example). From that we can then update the current responsiveness to take those changes into account.

The actual implementation will not be hard. The hardest part is to determine the expected behavior.

timvandermeij commented 9 years ago

We have had some discussion with the UX team on IRC. Below is the conversation for completeness.

*Thunderforge* Hi everyone. There has been a request for help from the UX team regarding the PDF viewer on small screens (via the PDF.js project issue #6191): https://github.com/mozilla/pdf.js/issues/6191
*Thunderforge* It has been noticed that on small screens, like mobile devices, and when browser-level zoom is used (for accessibility) that the find functionality cannot be used to search through PDFs.
*Thunderforge* In discussing the bug, there was some debate as to what buttons were actually necessary and if it was advisable to allow for find functionality at that small size.
*Thunderforge* One of the collaborators suggested contacting the UX team for help. So here I am doing that. I'm not sure what the proper procedure for this would be, but I am interested in moving forward with this issue as it is an accessibility concern.
*antlam* thunderforge: what’s the bug number?
*Thunderforge* PDF.js #6191 https://github.com/mozilla/pdf.js/issues/6191
*Thunderforge* That bug discusses it on small screen sizes. There is a duplicate linked to that issue that discusses it in relation to browser-level zoom.
*timvandermeij* Some context on the above: The general issue here is that we (the PDF.js team) need some advise on the best way of handling responsiveness for the PDF viewer. If you open https://mozilla.github.io/pdf.js/web/viewer.html and make the browser width smaller, you will see that the viewer is responsive, but  at very small widths buttons (like the Find button) disappear because there is simply no space. We would like to hear from the UX team: given the full toolbar, which buttons would you definitely keep at small width and which ones can disappear/be put in the >> menu?
*Thunderforge* Our concern was that some items are sent to the >> menu where they are still accessible (e.g. Download, Print). Others disappear, but have an alternate way to use their functionality (zooming can still be done with +/- buttons when the dropdown goes away). Find is the only feature that is completely inaccessible on small screen sizes, either through the >> menu or an alternate means, and cannot be accessed via keyboard shortcut when the button 
*Thunderforge* And thank you for the additional context, @timvandermeij
*timvandermeij* If the UX team thinks that the current responsiveness is fine, then we can find a way to add the Find button and fix some existing overlap issues. If not, then we would like to hear what should be adjusted to improve UX.
*timvandermeij* shorlander: Do you perhaps have ideas regarding this, as you made the original design of the viewer?
*shorlander* timvandermeij: off the top of my head, no. That kind of responsiveness wasn't a consideration at the time.
*timvandermeij* shorlander: Okay, so should we make our own decisions regarding the responsiveness of the toolbar or do you have any advice that we should stick to?
*timvandermeij* We are fine with making the required changes, we just want to make sure that the UX team agrees too and does not find that we have bad UX at the moment, because then we can address that at the same time.
*shorlander* timvandermeij, Thunderforge: I don't know why Find would need to disappear completely and not go into the overflow menu
*shorlander* As long as there is still a way to dismiss it
*Thunderforge* In other words, could it just stay where it is, even at small screen sizes, rather than disappearing completely or being moved into the overflow menu?
*shorlander* Thunderforge: I would prioritize "Find" over the zoom drop-down 
*Thunderforge* shorlander: Good to know. If we take out the zoom dropdown and would need even more space, would it be acceptable to have the next/previous buttons disappear, since there are still other ways to navigate?
*Thunderforge* The other ways being scrolling or the page number field.
*shorlander* Thunderforge: yeah, I can't imagine too many people are navigating with those buttons vs. finger scrolling.
*shorlander* But that's just a guess
*shorlander* Button targets are pretty small on a mobile device
*Thunderforge* shorlander: Good point.

I think we can summarize the required changes to improve the overall responsiveness as follows:

After those changes, the responsiveness and accessibility are much better and most open issues regarding this are probably fixed at the same time. If someone is willing to work on this, please feel free to create a PR to iterate on the issue.

Thunderforge commented 9 years ago

I am interested in working on this. It's the end of the work day in my time zone, but I hope to start a pull request tomorrow that addresses this.

timvandermeij commented 9 years ago

@Thunderforge Thank you! Improving the responsiveness would be great.

Snuffleupagus commented 9 years ago

Keep the Find button on the toolbar at all times.

One thing to keep in mind is that the Find button (and the PDF.js findbar) normally isn't used in Firefox (i.e FIREFOX and MOZCENTRAL builds), so we need to ensure that it still looks/works well there.

Thunderforge commented 9 years ago

@Snuffleupagus How would we test the lack of a Find button on FIREFOX and MOZCENTRAL builds?

Snuffleupagus commented 9 years ago

How would we test the lack of a Find button on FIREFOX and MOZCENTRAL builds?

Locally, you run node make firefox, and then install the addon (build/firefox/pdf.js.xpi) in Firefox.

gsampath127 commented 9 years ago

Thank you all !!! For quick response. An added note in mobiles , when the pdf link is given as source to iframe/frame the rendering behavior is same as desktop.

raviiii1 commented 9 years ago

I have tried to solve the issue with the find tool, and I have come up with this : capture

This is a two line pop-up for the find tool for small windows.For desktop or larger screens it retains its original form i.e. one line pop-up.Every thing on the find tool works as usual.There may be some standard dimensions that need to be adjusted here. So please suggest.

Please note that i have added the 'hiddenSmallView' class to the previous and next page arrow buttons, so they disappear for smaller screen sizes. This solves the problem of 'zoom out' and 'total page count' overlap problem, at least for 'English' language and up to 3-digit page counts. I haven't tried changing the language.

Please Note: I am a newbie and I don't no how to commit changes that I have made on my local copy of code to the Github repo, so PLEASE suggest any resource.I am open to any kind of advice. You can mail me at ravi.prakash0303@gmail.com.

Snuffleupagus commented 9 years ago

@raviiii1 Thanks for your interest; however there is already a work-in-progress patch submitted, please see PR #6274.