Skip to content

[pointer_interceptor] Remove iOS and Android from example #442

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

Merged

Conversation

stuartmorgan-g
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g commented Aug 18, 2021

The package is web-only, and the example doesn't build for other platforms,
so it shouldn't be configured for them.

This may be a point of user confusion, but it also fails if we fix the
repo script to build example apps (which it currently isn't, I believe
due to a long-ago regression that hadn't been noticed).

Pre-launch Checklist

  • The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
  • I updated CHANGELOG.md to add a description of the change.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

The package is web-only, and doesn't build for other platforms, so the
example shouldn't be configured for them.

This may be a point of user confusion, but it also fails if we fix the
repo script to build example apps (which it currently isn't, I believe
due to a long-ago regression that hadn't been noticed).
Copy link
Member

@ditman ditman left a comment

Choose a reason for hiding this comment

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

The package is web-only, and doesn't build for other platforms

The package is only active in the web, but it should compile (and noop) in mobile platforms (see the conditional export here)

I remember running the example app on Android a while back, but maybe it has bit rotted since then? :S

@stuartmorgan-g
Copy link
Contributor Author

Hm, I'll take another look. IIRC it was complaining about dart:html when I ran flutter/plugins#4248 locally.

@ditman
Copy link
Member

ditman commented Aug 27, 2021

Hm, I'll take another look. IIRC it was complaining about dart:html when I ran flutter/plugins#4248 locally.

Thanks for taking another look, I might have screwed up the example since I've written it... or maybe its integration tests :S

@stuartmorgan-g
Copy link
Contributor Author

lib/main.dart:6:8: Error: Not found: 'dart:html'
import 'dart:html' as html;
       ^
lib/main.dart:18:1: Error: Type 'html.Element' not found.
html.Element htmlElement = html.DivElement()
^^^^^^^^^^^^
lib/main.dart:18:6: Error: 'Element' isn't a type.
html.Element htmlElement = html.DivElement()
     ^^^^^^^
lib/main.dart:18:33: Error: Method not found: 'DivElement'.
html.Element htmlElement = html.DivElement()
                                ^^^^^^^^^^
lib/main.dart:204:18: Error: Constant evaluation error:
    return const HtmlElementView(
                 ^
../../../../flutter/packages/flutter/lib/src/widgets/platform_view.dart:350:15: Context: This assertion failed with message: HtmlElementView is only available on Flutter Web.
       assert(kIsWeb, 'HtmlElementView is only available on Flutter Web.'),

see the conditional export here

That's not the problem, this is

I remember running the example app on Android a while back, but maybe it has bit rotted since then? :S

That line dates back to #256 so I'm not sure how you could ever have run the example on anything but web.

So do you want to rewrite the example, or move forward with this PR?

@ditman
Copy link
Member

ditman commented Aug 31, 2021

Gosh, you're right, that's bad...

So do you want to rewrite the example, or move forward with this PR?

Nah, let's move on with the PR and I'll look at adding an Android version later, since I need to reshuffle a bunch of code to make it work :/

@ditman ditman merged commit 5c69b36 into flutter:master Aug 31, 2021
@stuartmorgan-g stuartmorgan-g deleted the pointer-interceptor-example-platforms branch August 31, 2021 17:35
austinstoker pushed a commit to austinstoker/packages that referenced this pull request Apr 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants