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

Add more zoom levels, like 133% #6943

Closed
bbondy opened this issue Nov 15, 2019 · 10 comments · Fixed by brave/brave-core#4092
Closed

Add more zoom levels, like 133% #6943

bbondy opened this issue Nov 15, 2019 · 10 comments · Fixed by brave/brave-core#4092

Comments

@bbondy
Copy link
Member

bbondy commented Nov 15, 2019

Test plan

See brave/brave-core#4092

Description

Currently there's pretty big jumps in the zoom levels, going from 125% to 150% for example, so users want other zoom levels in between like 133%.

For reference:
https://www.reddit.com/r/IAmA/comments/dwfbmf/im_brendan_eich_inventor_of_javascript_and/f7karn6/

This matches Chrome but it's some low hanging fruit and should be an easy change.

Steps to Reproduce

  1. Go to the hamburger menu and keep pressing the + button for the zoom level

Actual result:

It skips from 125% to 150%

Expected result:

More zoom levels, like 133%

Reproduces how often:

Always

Brave version (brave://version info)

All

Version/Channel Information:

  • Can you reproduce this issue with the current release? Yes
  • Can you reproduce this issue with the beta channel? Yes
  • Can you reproduce this issue with the dev channel? Yes
  • Can you reproduce this issue with the nightly channel? Yes

Other Additional Information:

  • Does the issue resolve itself when disabling Brave Shields? No
  • Does the issue resolve itself when disabling Brave Rewards? No
  • Is the issue reproducible on the latest version of Chrome? Yes

Miscellaneous Information:

@bbondy bbondy added the priority/P3 The next thing for us to work on. It'll ride the trains. label Nov 15, 2019
@fow5040
Copy link

fow5040 commented Nov 23, 2019

I assume this won't be too hard - I'd like to get into supporting this project so would like to grab this

@fow5040
Copy link

fow5040 commented Nov 23, 2019

edit: Changes need to be made in brave-core to adjust default zoom levels

@fow5040
Copy link

fow5040 commented Nov 27, 2019

Fix was trivial but involved creating patches to update constants in chromium source code

brave/brave-core#4092

@bsclifton
Copy link
Member

Created https://bugs.chromium.org/p/chromium/issues/detail?id=1030344 to upstream a proper fix for getting the constants in one place (using knowledge gained when helping with brave/brave-core#4092)

@bsclifton
Copy link
Member

bsclifton commented Dec 3, 2019

Upstreamed a fix that will make changing this very easy (meaning we can remove the overrides if it's accepted):
https://chromium-review.googlesource.com/c/chromium/src/+/1949552

@bsclifton
Copy link
Member

And here's the accepted fix, merged into upstream Chromium! 😄
https://chromium.googlesource.com/chromium/src/+/08b57c09851e2bdcb39ea70e46707f3bab6a0a4a

Once we're using this version (master/Canary is 81), we can revert brave/brave-core#4092 except for chromium_src/components/zoom/page_zoom_constants.cc and we should be good 😄

@fow5040
Copy link

fow5040 commented Dec 18, 2019

awesome!

@LaurenWags
Copy link
Member

@bsclifton question about the test plan - on step 6, should I be zooming on the Print Preview dialog? the macOS dialog print preview for PDF does have a Custom scale section, but it defaults to 100 and isn't limited to the browser zoom settings 110, 125, 133, etc. I set it to 103 below just to show that it increments/decrements by 1.

Screen Shot 2020-01-07 at 11 26 38 AM

@bsclifton
Copy link
Member

@LaurenWags ah - good catch there. Testing that is not trivial, which I found out later when comitting some related changes upstream

When you're on that page, the scale is not affecting zoom. You have to use the mouse to hover over the left and use the + and - buttons. Unfortunately, it doesn't say the %

However, you can right click the preview itself and then pick Inspect. It'll open a window and from there, you can pick Console. In the console, type in window.viewer.viewport_.internalZoom_ and it should show the %

Let me update the test plan...

@GeetaSarvadnya
Copy link

GeetaSarvadnya commented Jan 9, 2020

Verification passed on

Brave 1.3.87 Chromium: 79.0.3945.117 (Official Build) beta (64-bit)
Revision 04f0a055010adab4484f7497fbfdbf312c307f1d-refs/branch-heads/3945@{#1019}
OS Windows 10 OS Version 1803 (Build 17134.1006)

image

Verified passed with

Brave 1.3.87 Chromium: 79.0.3945.117 (Official Build) beta (64-bit)
Revision 04f0a055010adab4484f7497fbfdbf312c307f1d-refs/branch-heads/3945@{#1019}
OS macOS Version 10.14.6 (Build 18G103)

Screen Shot 2020-01-09 at 12 42 16 PM

Screen Shot 2020-01-09 at 12 48 49 PM

Screen Shot 2020-01-09 at 12 50 27 PM

  • Verified STR from description

Screen Shot 2020-01-09 at 12 43 30 PM

Screen Shot 2020-01-09 at 12 43 36 PM

Verification passed on

Brave 1.3.87 Chromium: 79.0.3945.117 (Official Build) beta (64-bit)
Revision 04f0a055010adab4484f7497fbfdbf312c307f1d-refs/branch-heads/3945@{#1019}
OS Ubuntu 18.04 LTS

image
image
image

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

Successfully merging a pull request may close this issue.

6 participants