Fixed ctrl+wheel zooming in reader mode#2189
Open
Nassim-Saboundji wants to merge 1 commit intominbrowser:masterfrom
Open
Fixed ctrl+wheel zooming in reader mode#2189Nassim-Saboundji wants to merge 1 commit intominbrowser:masterfrom
Nassim-Saboundji wants to merge 1 commit intominbrowser:masterfrom
Conversation
Collaborator
|
This is a neat approach, but I think we should investigate first why ctrl+scroll inside an iframe doesn't work - I would expect that to zoom the whole page in or out (on any site, not just reader mode). The way that's implemented is that there's a preload script that runs inside the webpage that sends mouse events to the UI process: And then in the parent process, it listens for those events and zooms the page: My guess is that the mouse events aren't being forwarded from the iframe correctly somehow, but I haven't looked into it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A fix for : 'Reader View' zoom in / out via ctrl + mouse wheel or ctrl++ / ctrl+- not working. However it only addresses ctrl + mouse wheel zooming.
Caveat
Not sure how this will behave in MacOS which uses the Cmd key instead of Ctrl. Do MacOS browsers make the substitution automatically or this case needs to be handled in code as well?
I could use e.metaKey to detect the Cmd key on MacOS but it will return true if the windows key is pressed on Windows.
How it looks like in practice
8mb.video-li1-p22YmFIV.mp4
Reasoning behind the fix
The issue was that you can't zoom/de-zoom the page if the cursor is above the iframe which contains the article. I first tried
to write some logic to zoom de-zoom the iframe itself.
The issue is that the article would indeed zoom/de-zoom but not the rest of the page which looked jarring.
I finally ended up creating a transparent div that I put above the iframe. If the user presses the Ctrl key we set pointer-events property of the transparent div to none. This way the cursor interacts with the div and not the iframe. This allows the user to leverage zooming/dezooming the same way you would normally on a web page.
Once the user stops pressing Ctrl we set the pointer-events property to auto which allows the user the interact with the iframe beneath the div. This way the user can click on links, highlight text, etc...
Tests
I only tested this on my local windows machine.