SAP / openui5

OpenUI5 lets you build enterprise-ready web applications, responsive to all devices, running on almost any browser of your choice.
http://openui5.org
Apache License 2.0
2.97k stars 1.24k forks source link

PDF viewer - emdedded doesn't work on mobile devices #1759

Closed jobberwalker closed 6 years ago

jobberwalker commented 7 years ago

Hello, The PDF embedded view does not work on mobile devices, checked on ipad, nexus as well as on desktop chrome with mobile devices simulation. The PDF is simply not visible.

OpenUI5 version: 1.48.11 Browser/version (+device/version): Chrome (recent version)

Steps to reproduce the problem:

  1. open on phone https://sapui5.hana.ondemand.com/#/sample/sap.m.sample.PDFViewerEmbedded/preview
  2. the embedded pdf does not show.
stephania87 commented 7 years ago

@jobberwalker The PDF Viewer uses an iframe to load the PDF document in browser, where the browser's PDF plugin handles the document. If the document is not displayed, try to configure the browser's PDF plugin, or use the PDFViewer control;s download button (the down arrow).

I am logging the case for verification with ticket #1770563168. Updates will be posted here as well.

jobberwalker commented 7 years ago

Hello, Chrome has a built-in plugin for PDFs. It works in desktop mode but when you open on mobile it does not. Even if you simulate mobile using dev tools and enable mobile devices (see below example for ipad) PS. I have also checked on real ipad on chrome.

screenshot

bkrrrr commented 7 years ago

Hi,

that's actually somehow the desired behavior for mobile devices. PDFs should be opened as "new tab". By doing so, the device can handle it with his native viewer.

Just open a random pdf (google .pdf) on a android 7 device. There is no browser support, a native app will be used. There was/is native support for iOS.

obohaciak commented 7 years ago

Hi all,

Correct, this is the designed behavior.

On mobile devices (smartphones and tablets), the PDF viewer control renders a toolbar with the title and a download icon, which behaves as a standard device/browser file link. We've made this note in Fiori Design Guidelines 1.48 and we'll add it to the SDK documentation.

While aiming for consistent behavior and experience across mobile devices our team realized that there's no foolproof way to determine whether a given platform, OS, form factor, version would be capable of opening the PDF file embedded in the HTML document as is the case on desktop devics. We've created a matrix of all supported browsers and devices to determine if they're capable of displaying the document embedded and found the lowest common denominator giving a consistent behavior is having a download link.

We also considered providing a completely JS based PDF renderer, but that would have meant digging ourselves deeper into the hole of browser bugs & quirks and that's not the direction we wanted to take with SAPUI5.

We're however open to ideas that would improve on the current design. Your suggestions are much appreciated!

O

codeworrior commented 7 years ago

What about an option to force the PDFViewer into embedded mode also on mobile devices? Could make sense if an app is created only for a limited group of users where devices / browsers and their capabilities are known (e.g. inside a company)? Default for sure would be 'false' on mobile.

And what about publishing the matrix that you mentioned (as a kind of 'can i use' and only as an indicator - consumers always should test for their specific use cases) ?

Just an idea...

jobberwalker commented 7 years ago

Hello, I would simply leave this decision to developer and steer by object property. Nowadays phones are having hi res displays and embedded preview (if for example pdf doc was rendered correctly) would be very helpful. JB

obohaciak commented 7 years ago

Thanks for the feedback. I asked our devs to see how feasible this could be. We shall soon know if we can provide an override API that would make the control ignore the user agent/device and try to force open the default PDF viewer inside the browser,

springframeworkpguru commented 6 years ago

Hello, Are there any other ways I can show PDF inside a mobile viewer. I used PDF.js, but it takes only pdf file path as a parameter. In my case, I will be downloading PDF file from the server (.xml path) and want to display it using "inline=1".

bkrrrr commented 6 years ago

Just to add another comment.

It is very frustrating that the ui5-pdf-viewer can`t tell me, if its rendering a PDF. I would prefer an "try rendering and tell me if it doesn't work" approach.

springframeworkpguru commented 6 years ago

Hi obohaciak, Any updates on PDFViewer showing PDF inside mobile browser? When can we expect this.

wombling commented 6 years ago

Noting that it is required to set the height of the PDF viewer, it would be very helpful if there was some way that could be used to use an alternate height and control if browser support failed.

something like:

 <PDFViewer source="{/source}" title="{/title}" height="{/height}" 
      failToRenderHeight="10rem">
   <failToRenderAlternative>
    <Link text="Download/Open PDF" href="{/source} />
  </failToRenderAlternative>
 </PDFViewer>

having a alternate view on mobile devices is cool, but since control cannot resize itself to fit the rendered PDF there is a need to set this - and no point developers having to re-invent and redo the "PDF is gonna fail to render" code outside the control if the control is already doing it.

tkolmacka commented 6 years ago

Hello all,

I added new property displayType to specify how PDFViewer displays a PDF file. There are 3 options (Auto, Link, Embedded). Auto is the current behavior, where it switches between a link and embedded based on device type. Link will display toolbar with button to download or show PDF in new tab. Embedded attempts to show PDF in embedded mode regardless of device type. Please keep in mind that many mobile browsers will not display PDF files in an embedded mode. If a device doesn’t support embedded mode, error will be display and in log will be warning message.

You can try the behavior yourself with my fork Your feedback is appreciated.

obohaciak commented 6 years ago

Thanks Tom for adding this!

boghyon commented 6 years ago

Thanks @tkolmacka. It would be nice though if the initial letter of each enum value is capitalized, same as the key name. Currently, they're all in lowercase. https://github.com/SAP/openui5/blob/825cae63028c2333e1f4f49d77e41daccc57f7b7/src/sap.m/src/sap/m/library.js#L2487-L2510

According to the doc:

The value for each key must be a string literal, equal to the key itself.

This also makes defining the right enum value more intuitive in XML views. Otherwise, there is no way for an app developer to find out that the value must be in lowercase without looking at the source code. displayType="Embedded" won't work (and doesn't even throw expected error) whereas displayType="embedded" is valid.

obohaciak commented 6 years ago

You're right @boghyon, well spotted. The enum values should start with uppercase and the enum's name should be singular. We'll get that fixed, I've opened a followup issue #2162 since this one is already closed.