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

API Review: IsSwipeNavigationEnabled.md #1400

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 96 additions & 0 deletions specs/IsSwipeNavigationEnabled.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
# Background

Swiping navigation on touch screen includes:
1. Swipe left/right (swipe horizontally) to navigate to previous/next page in navigation history.
1. Pull to refresh (swipe vertically) the current page.

In response to customer requests, the WebView2 team has introduced this Swipe Navigation API which allows developers to disable or enable the the ability of end users to use swiping gesture to navigate in WebView via a setting.

In this document we describe the new setting. We'd appreciate your feedback.

# Description
The default value for `IsSwipeNavigationEnabled` is `TRUE`.
When this setting is set to `FALSE`, it disables the ability of the end users to use swiping gesture on touch input enabled devices to trigger navigations such as swipe left/right to go back/forward or pull to refresh current page.
Copy link
Contributor

Choose a reason for hiding this comment

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

Navigation and refresh are distinct, apps might want to disable one but enable the other. Why block out that scenario?

Copy link
Contributor

Choose a reason for hiding this comment

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

@tofuandeve Do we have customers using pull to refresh? Do we expect this could be a problem in the future?

Also, is it implemented this way because chromium has just one setting to control both back/forward swipes and refresh swipes?

We might say that since pull to refresh is off by default and not exposed via our API currently, that if we receive feedback about it, we can add a IsPullToRefreshEnabled setting that is not impacted by the setting we're adding now.

Copy link
Contributor

Choose a reason for hiding this comment

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

Navigation and refresh are distinct

That's why I asked the clarifying question later about whether "Refresh" is considered a navigation by WebView2. The answer is "Yes" (effectively "navigate to the same place you already are"), in which case saying just "navigation" is internally consistent with the rest of WebView2.

So there's this general concept of navigation in WebView2, with different flavors based on the effect on the navigation history.

  • Back: Pop off the navigation history.
  • Forward: Push onto the navigation history.
  • Refresh: No change to navigation history.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@MikeHillberg Refresh is considered a navigation in WebView2. As @david-risney mentioned above, swipe left/right to go back/forward and pull to refresh are bundled up together under overscroll controler in Chromium. We currently have some customers asked to be able to disable both swipe and pull to refresh.

Copy link
Contributor

Choose a reason for hiding this comment

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

It could be too boolean properties though, which would make it possible to disable both or just one. I'm not trying to add features, but worried that someone will ask for only one and this API won't allow for it.

Copy link
Contributor

Choose a reason for hiding this comment

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

So right now IsSwipeNavigationEnabled will disable PullToRefresh, and then maybe in the future it won't. Is that the kind of behavior breaking change we can't make?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@david-risney hm, I need your thoughts on this please?

Copy link
Contributor

Choose a reason for hiding this comment

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

Pull to refresh isn't in our API surface yet. Its off by default and the end dev needs to use a command line switch to enable it. Its not really discoverable and I'm not worried about high adoption of that feature. Regardless we can support compat for this in the future if we want to properly support pull to refresh we can add a bool CoreWebView2Settings.IsPullToRefreshEnabled (that defaults to false). We can have the command line switch work as is (IsSwipeNavigationEnabled affects it) and the new actual API we add IsPullToRefreshEnabled is independent of IsSwipeNavigationEnabled.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry if I'm not following, but it sounds like you're still saying that IsSwipeNavigationEnabled today will affect PTR. But in the future we might change that behavior, it will no longer affect PTR, and if want to control PRT plesae use IsPullToRefreshEnabled.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm saying PTR is disabled by default. Today if you use the command line to enable PTR you also need to set IsSwipeNavigationEnabled to true. If we add a new API to enable PTR, that new API doesn't have to require the command line or IsSwipeNavigationEnabled being set.

Disabling/Enabling `IsSwipeNavigationEnabled` does not take effect until the next navigation.


# Examples
```cpp
wil::com_ptr<ICoreWebView2> webView;
void SettingsComponent::ToggleSwipeNavigationEnabled()
{
// Get webView's current settings
wil::com_ptr<ICoreWebView2Settings> coreWebView2Settings;
CHECK_FAILURE(webView->get_Settings(&coreWebView2Settings));

wil::com_ptr<ICoreWebView2Settings6> coreWebView2Settings6;
coreWebView2Settings6 = coreWebView2Settings.try_query<ICoreWebView2Settings6>();
if(coreWebView2Settings6)
{
BOOL enabled;
CHECK_FAILURE(coreWebView2Settings6->get_IsSwipeNavigationEnabled(&enabled));
CHECK_FAILURE(coreWebView2Settings6->put_IsSwipeNavigationEnabled(enabled ? FALSE : TRUE));
}
}
```

```c#
private WebView2 _webView;
void ToggleSwipeNavigationEnabled()
{
var coreWebView2Settings = _webView.CoreWebView2.Settings;
coreWebView2Settings.IsSwipeNavigationEnabled = !coreWebView2Settings.IsSwipeNavigationEnabled;
}
```

# Remarks
This API only affects the decision whether to trigger navigations of the main html page when end users perform overscrolling motions by swiping and has no effect on the scrolling interaction used to explore the web content shown in WebView2 or the CSS overscroll-behavior property.

# API Notes

See [API Details](#api-details) section below for API reference.

# API Details

## Win32 C++
```cpp
[uuid(45b1f964-f703-47ac-a19a-b589dd0c5559), object, pointer_default(unique)]
interface ICoreWebView2Settings6 : ICoreWebView2Settings5 {
/// Swiping gesture navigation on touch screen includes:
/// 1. Swipe left/right (swipe horizontally) to navigate to previous/next
/// page in navigation history.
/// 2. Pull to refresh (swipe vertically) the current page. (This feature is
/// currently disabled by default in the browser, to enable in WebView2, use
/// `put_AdditionalBrowserArguments` api with "--pull-to-refresh" switch).
tofuandeve marked this conversation as resolved.
Show resolved Hide resolved
///
/// The `IsSwipeNavigationEnabled` property enables or disables the ability of the
/// end user to use swiping gesture on touch input enabled devices to
/// navigate in WebView2. It defaults to `TRUE`.
/// When set to `FALSE`, the end user cannot swipe to navigate or pull to refresh.
/// This API only affects the overscrolling navigation functionality and has no
/// effect on the scrolling interaction used to explore the web content shown
/// in WebView2.
/// Disabling/Enabling IsSwipeNavigationEnabled does not take effect until the
/// next navigation.
///
/// \snippet SettingsComponent.cpp ToggleSwipeNavigationEnabled
[propget] HRESULT IsSwipeNavigationEnabled([out, retval] BOOL* enabled);
/// Set the `IsSwipeNavigationEnabled` property
[propput] HRESULT IsSwipeNavigationEnabled([in] BOOL enabled);
}
```

## .NET and WinRT

```c#
namespace Microsoft.Web.WebView2.Core
{
public partial class CoreWebView2Settings
{

public bool IsSwipeNavigationEnabled { get; set; };

}
}

```