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.

Sign up for GitHub

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

Chore: isMobile refactor #35

Merged
merged 2 commits into from
Apr 3, 2017
Merged

Chore: isMobile refactor #35

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a7852cd
Chore: move isMobile check to user agent and add check back to downlo…
Mar 29, 2017
f4132ae
Merge branch 'master' into refactor
Apr 3, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/lib/Browser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,19 +219,19 @@ class Browser {
}

/**
* Returns whether the browser is a mobile browser. Taken from Modernizr:
* https://github.com/Modernizr/Modernizr/blob/master/feature-detects/touchevents.js
* Returns whether the browser is a mobile browser.
*
* @return {boolean} true if browser supports download
*/
static isMobile() {
return (('ontouchstart' in window) || (window.DocumentTouch && document instanceof DocumentTouch));
// Relying on the user agent to avoid desktop browsers on machines with touch screens.
return /iphone|ipad|ipod|android|blackberry|bb10|mini|windows\sce|palm/i.test(navigator.userAgent);
}

/**
* Returns whether the browser can download via HTML5. taken from Modernizr:
* https://github.com/Modernizr/Modernizr/blob/master/feature-detects/a/download.js
*
* Currently not supported by IE11 or Safari 10.0 http://caniuse.com/#feat=download
* @return {boolean} true if browser supports download
*/
static canDownload() {
Expand Down
5 changes: 3 additions & 2 deletions src/lib/Preview.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -915,10 +915,11 @@ class Preview extends EventEmitter {
*/
finishLoading(data = {}) {
// Show or hide annotate/print/download buttons
if (checkPermission(this.file, PERMISSION_DOWNLOAD) && this.options.showDownload && Browser.canDownload()) {
// canDownload is not supported by all of our browsers, so for now we need to check isMobile
if (checkPermission(this.file, PERMISSION_DOWNLOAD) && this.options.showDownload && (Browser.canDownload() || !Browser.isMobile())) {
showDownloadButton(this.download);

if (checkFeature(this.viewer, 'print')) {
if (checkFeature(this.viewer, 'print') && !Browser.isMobile()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this mobile check? We're already in a (canDownload() || !Browser.isMobile()) if block right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes because a browser like android chrome for example has the download attribute, but cannot print iFrames that we generate for text files.

showPrintButton(this.print);
}
}
Expand Down
9 changes: 9 additions & 0 deletions src/lib/__tests__/Preview-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1328,12 +1328,21 @@ describe('lib/Preview', () => {

it('should show download button if download is supported by browser', () => {
stubs.canDownload.returns(false);
stubs.isMobile.returns(true);

preview.finishLoading();
expect(stubs.showDownloadButton).to.not.be.called;

stubs.canDownload.returns(false);
stubs.isMobile.returns(false);

preview.finishLoading();
expect(stubs.showDownloadButton).to.be.called;


stubs.canDownload.returns(true);
stubs.isMobile.returns(false);


preview.finishLoading();
expect(stubs.showDownloadButton).to.be.calledWith(preview.download);
Expand Down