Skip to content

Conversation

@ditman
Copy link
Member

@ditman ditman commented Dec 14, 2020

This PR adds the (web) PointerInterceptor widget, to address issues like: flutter/flutter#54027.

The widget can be used simply as a wrapper of any widget that is overlaid on top of a platform view, so it can handle clicks "as expected".

More info about usage and testing in the corresponding READMEs!

The example app is deployed here running on top of flutter master: https://dit-mouse-boundary.web.app

@ditman ditman requested review from ferhatb and yjbanov December 14, 2020 20:22
@ditman
Copy link
Member Author

ditman commented Dec 14, 2020

In an offline discussion, this has been suggested to be renamed pointer_interceptor; I'll get that done. (Done)

@ditman ditman changed the title [web] Add MouseClickBoundary widget [web] Add PointerInterceptor widget Dec 15, 2020
@ditman
Copy link
Member Author

ditman commented Dec 16, 2020

I think I've addressed all of @yjbanov's comments. I also migrated the package to Null-Safety by bumping its lower SDK version (and some minimal changes to the code).

Attempting to migrate tests now (but I don't think this'll work)

@ditman
Copy link
Member Author

ditman commented Dec 16, 2020

Reverted null safety because of:

Checking that web_pointer_interceptor can be published.
Package validation found the following potential issues:
* Packages with an SDK constraint on a pre-release of the Dart SDK should themselves be published as a pre-release version. If this package needs Dart version 2.12.0-0, consider publishing the package as a pre-release instead.
  See https://dart.dev/tools/pub/publishing#publishing-prereleases For more information on pre-releases.
* This package is opting into null-safety, but a dependency or file is not.
  
  package:sky_engine is not opted into null safety in its pubspec.yaml:
    ╷
  8 │   sdk: '>=1.11.0 <3.0.0'
    │        ^^^^^^^^^^^^^^^^^
    ╵

@ditman ditman requested a review from yjbanov December 16, 2020 02:38
Comment on lines 11 to 16
<center>

![In the dashed areas, clicks won't work](doc/img/affected-areas.png)

_In the dashed areas, clicks won't work as expected._
</center>
Copy link
Member Author

Choose a reason for hiding this comment

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

I'll tweak this after pushing, when the images live in the repo.

Copy link
Contributor

@yjbanov yjbanov left a comment

Choose a reason for hiding this comment

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

lgtm

@override
Widget build(BuildContext context) {
return Scaffold(
appBar: AppBar(
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the AppBar or its contents be wrapped in PointerInterceptor? The content in the body can be rendered under the appbar and if it contains iframes, consume pointer events.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, you're right, and that'd also cover the Drawer open button, I'm going to give that a shot!

Copy link
Member Author

Choose a reason for hiding this comment

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

Oooops, not for now:

The argument type 'PointerInterceptor' can't be assigned to the parameter type 'PreferredSizeWidget'.

The contents should be wrappable (individually)

@ditman ditman merged commit 1c117bb into flutter:master Dec 16, 2020
@ditman ditman deleted the web-click-boundary branch December 17, 2020 00:48
@RandalSchwartz
Copy link

RandalSchwartz commented Dec 17, 2020

It's not clear how this differs from AbsorbPointer or IgnorePointer. Wouldn't this functionality be better in those classes?
Or if it needs platform-specific code, hide it behind a conditional import?

@ditman
Copy link
Member Author

ditman commented Dec 17, 2020

Wouldn't this functionality be better in those classes? Or if it needs platform-specific code, hide it behind a conditional import?

@RandalSchwartz I've double-checked the documentation and it seems that this doesn't fit well with neither Ignore/AbsorbPointer.

The issue, in web, is that any flutter element that you overlay on top of an HtmlElementView will behave as if it was wrapped in IgnorePointer: clicks won't land on them, but will go through to the underlying view (there's an example here, the pink webview will get the clicks that you do on the left button. The right button is wrapped in PointerInterceptor and will work as expected).

This happens because in the browser, it seems that the HtmlElementView is handling (and consuming) the mouse events before Flutter's glass-pane is able to handle them (and decide whether to use Ignore/AbsorbPointer or anything else!).

In this case, the PointerInterceptor is just adding an empty HtmlElementView under the wrapped child that prevents the underlying HtmlElementView from seeing any events in that covered region, allowing Flutter to handle the event of the child.

I'm not 100% sure that this would mesh well with either Absorb/IgnorePointer. WDYT?

I also added something similar to the above in the README of the PointerInterceptor, do you think it needs further elaboration? More details can be added there.

austinstoker pushed a commit to austinstoker/packages that referenced this pull request Apr 29, 2022
[Migrate & Simplify] bottom navigation multiple beamers example
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants