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

Add new method to set layout-based zoom level limit #8041

Merged
merged 9 commits into from Nov 22, 2016

Conversation

Projects
None yet
3 participants
@paulcbetts
Contributor

paulcbetts commented Nov 21, 2016

The current behavior of setZoomLevelLimits affects the visual-based zoom (i.e. no layout changes) despite all of the rest of the methods affecting layout-based zoom. While this zoom survives in-page navigations such as iframes being loaded, it also causes pages to be cut-off.

The ideal fix for this would to be to just change the behavior of this method but that would be a breaking change. This PR introduces a new zoom level limit method to limit layout-based zoom.

This allows you to work around #6958 by doing the following:

function setZoomHarder(wv, zoomLevel) {
  wv.setLayoutZoomLevelLimits(zoomLevel, zoomLevel);
  wv.setZoomLevel(zoomLevel);
}

Refs #6958

paulcbetts added some commits Nov 21, 2016

@kevinsawicki

This comment has been minimized.

Show comment
Hide comment
@kevinsawicki

kevinsawicki Nov 21, 2016

Contributor

The ideal fix for this would to be to just change the behavior of this method but that would be a breaking change.

Any feels about changing the current method to accept a third type parameter so it could be something like?

wv.setZoomLevelLimits(0, 3, 'layout'); // new behavior
wv.setZoomLevelLimits(1, 2, 'visual'); // default behavior if unspecified
Contributor

kevinsawicki commented Nov 21, 2016

The ideal fix for this would to be to just change the behavior of this method but that would be a breaking change.

Any feels about changing the current method to accept a third type parameter so it could be something like?

wv.setZoomLevelLimits(0, 3, 'layout'); // new behavior
wv.setZoomLevelLimits(1, 2, 'visual'); // default behavior if unspecified
@paulcbetts

This comment has been minimized.

Show comment
Hide comment
@paulcbetts

paulcbetts Nov 22, 2016

Contributor

@kevinsawicki Feels a little weird to me but if you think it's better I'm okay with that

Contributor

paulcbetts commented Nov 22, 2016

@kevinsawicki Feels a little weird to me but if you think it's better I'm okay with that

@kevinsawicki

This comment has been minimized.

Show comment
Hide comment
@kevinsawicki

kevinsawicki Nov 22, 2016

Contributor

I pushed a few changes to this branch:

  • Expose and document setVisualZoomLeveLimits as public on webContents, webFrame, and <webview>
  • Map setZoomLevelLimits to SetVisualZoomLevelLimits for backwards compatibility
  • Add notes to planned-breaking-changes.md about setZoomLevelLimits
Contributor

kevinsawicki commented Nov 22, 2016

I pushed a few changes to this branch:

  • Expose and document setVisualZoomLeveLimits as public on webContents, webFrame, and <webview>
  • Map setZoomLevelLimits to SetVisualZoomLevelLimits for backwards compatibility
  • Add notes to planned-breaking-changes.md about setZoomLevelLimits

@kevinsawicki kevinsawicki merged commit 95ab481 into master Nov 22, 2016

8 of 9 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
electron-linux-arm Build #4755203 succeeded in 64s
Details
electron-linux-ia32 Build #4755204 succeeded in 58s
Details
electron-linux-x64 Build #4755205 succeeded in 124s
Details
electron-mas-x64 Build #2885 succeeded in 7 min 47 sec
Details
electron-osx-x64 Build #2894 succeeded in 7 min 30 sec
Details
electron-win-ia32 Build #1953 succeeded in 10 min
Details
electron-win-x64 Build #1929 succeeded in 11 min
Details

@kevinsawicki kevinsawicki deleted the webview-zoom-factor branch Nov 22, 2016

@jwheare jwheare referenced this pull request Feb 1, 2017

Closed

Half Zoom #67

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment