WPF ZoomIn/Out Functionality #306
Conversation
Added GetZoomLevel and SetZoomLevel methods to ManagedCefBrowserAdapater Currently unable to get GetZoomLevel to return any value other than 0, comment in source says "This method can only be called on the UI thread", so need to investigate that further. SetZoomLevel works as expected. Added WPF Commands for ZoomIn, ZoomOut and ZoomReset. Added Dependency properties for ZoomLevel and ZoomLevelIncrement Added content menu items for ZoomIn, ZoomOut and ZoomReset to Wpf Example
| { | ||
| auto cefHost = _renderClientAdapter->TryGetCefHost(); | ||
|
|
||
| if (cefHost != nullptr) |
There was a problem hiding this comment.
The whitespace here is quite messed up. Could you take a look at it?
There was a problem hiding this comment.
Interesting! The code looks fine in visual studio, I can see the formatting of the patch is clearly all over the place. I'm fairly new to the world of Git, so if you have any insights they would be greatly appreciated, otherwise I'll take a look at the issue shortly.
There was a problem hiding this comment.
@amaitland we use spaces for whitespace here - not tabs. I guess that's the problem ... Take a look at your Visual Studio settings (don't know if it's configurable per solution/project). Otherwise I can reccomend the PowerCommands aka "Productivity Power Tools" VS add-in. It complains if a file has mixed tabs/spaces - and you just click a button to fix it in either direction.
There was a problem hiding this comment.
But be very careful to not commit a lot of whitespace changes along with other changes (which is easy if you click the "fix tabs/spaces" button 😃). Please fix as needed, but as separate pull requests.
There was a problem hiding this comment.
Makes perfect sense, thank you both for the prompt reply's. I've just committed the formatting changes.
Fix indentation from previous commit (change from tabs to spaces)
|
This is good, except for the whitespace mixup (should be space, not tab) and commented out code (which should be deleted or included, but not commented out). I suggest you make a new branch off of the current Let's start with that and I can then give the code a further review, once these more trivial settings are done. |
|
|
||
| protected virtual void OnZoomLevelChanged(double oldValue, double newValue) | ||
| { | ||
| if (!Cef.IsInitialized && !Cef.Initialize()) |
There was a problem hiding this comment.
I wonder if it's a good idea to implicitly initialize CEF when the zoom level is changed? Could it have "bad" implications?
There was a problem hiding this comment.
TBH that was copied from one of the other property change handlers. Just the check for IsInitialized should be enough, I'll update.
|
Gha, don't write comments about old, no-more-relevant changesets... 😄 I think this is all good, except for the commented out code. Please add a commit (to your |
|
I'll remove the commented out code and also the call to Cef.Initialize() in on OnZoomLevelChanged. Sorry about the whitespace problems, checking that sort of stuff is a total waste of your time, so my appologies, I'll look at sorting this issue out going forward! |
Remove call to Cef.Initialize() in OnZoomLevelChanged
|
Changes made, let me know if there is anything else. |
|
Anything else required before this one can be merged? |
| @@ -55,5 +69,11 @@ public interface IWpfWebBrowser : IWebBrowser | |||
| /// <returns><c>true</c> if keyboard focus and logical focus were set to this element; <c>false</c> if only logical focus | |||
| /// was set to this element, or if the call to this method did not force the focus to change.</returns> | |||
| bool Focus(); | |||
|
|
|||
| /// <summary> | |||
| /// The zoom level at which the browser control is currently displaying. Can be set to 0 to clear the zoom level(resets to default zoom level) | |||
There was a problem hiding this comment.
I lack a space here, but since this is such a minor change I'll fix it myself.
|
Looks good, merging. |
Added GetZoomLevel and SetZoomLevel methods to ManagedCefBrowserAdapater
Currently unable to get GetZoomLevel to return any value other than 0,
comment in source says "This method can only be called on the UI
thread", so need to investigate that further. SetZoomLevel works as
expected.
Added WPF Commands for ZoomIn, ZoomOut and ZoomReset.
Added Dependency properties for ZoomLevel and ZoomLevelIncrement
Added content menu items for ZoomIn, ZoomOut and ZoomReset to Wpf
Example