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

New: Add pseudo fullscreen to mobile Safari #246

Merged
merged 3 commits into from
Jul 31, 2017
Merged

New: Add pseudo fullscreen to mobile Safari #246

merged 3 commits into from
Jul 31, 2017

Conversation

jeremypress
Copy link
Contributor

  • Mobile Safari is the only browser we support that doesn't support the Fullscreen API. This change sets the content to fill the viewport by setting position: fixed
  • Added touch-action manipulation to prevent double-tap zoom on the content

@@ -361,6 +354,20 @@ class BaseViewer extends EventEmitter {
}

/**
* Applies appropriate styles and resizes the document depending on fullscreen state
* @protected
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit - space after description before @. We also stopped marking things as protected

@@ -361,7 +361,7 @@ class DocBaseViewer extends BaseViewer {
const previousPageButtonEl = this.containerEl.querySelector('.bp-previous-page');
const nextPageButtonEl = this.containerEl.querySelector('.bp-next-page');

// Safari disables keyboard input in fullscreen
// Safari disables keyboard input in fullscreen before Safari 10.1
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we detect this and re-enable page selector in Safari 10.1?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll do that in a separate PR since it's not trivial to select 10.1 on iOS and Safari at the same time.

@jeremypress jeremypress merged commit b8c776d into box:master Jul 31, 2017
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

Successfully merging this pull request may close these issues.

4 participants