API Review: IsSwipeNavigationEnabled.md #1400
Conversation
IsSwipeNavigationEnabled spec draft
|
|
||
| # 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. |
There was a problem hiding this comment.
Navigation and refresh are distinct, apps might want to disable one but enable the other. Why block out that scenario?
There was a problem hiding this comment.
@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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
@david-risney hm, I need your thoughts on this please?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
|
||
| # 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. |
There was a problem hiding this comment.
@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.
david-risney
added
the
review completed
This is a review for the new IsSwipeNavigationEnabled API.