Skip to content

[pointer_interceptor] Shadow dom tweaks #364

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
merged 3 commits into from
May 27, 2021

Conversation

ditman
Copy link
Member

@ditman ditman commented May 27, 2021

Change the pointer_interceptor so that it works better with the new slots way of embedding HtmlElementViews in flutter web master.

Related to: flutter/flutter#80524

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.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Copy link
Contributor

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

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

LGTM with a couple of questions.

..style.left = '0'
..style.position = 'relative';
..style.width = '100%'
..style.height = '100%';
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this change fine for stable as well? (If not, you could always update the Flutter SDK requirement so Flutter 2.2 won't versions that have this change.)

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, thanks for asking. I tested this both with stable and master, manually and with the driver tests.

This is backwards compatible, it's just that there's many ways to "cover a parent div" with CSS, but in the engine we only check "are style.width and style.height set (to any value?) else: warn user".

This code is to disable that warning from our own packages.

No need to bump a whole z version.
@abnerh69
Copy link

Is this chat still opened? I am trying to avoid reload of iframe url using master/stable channel but can't make it works with slot like you said. Please, help!

@stuartmorgan-g
Copy link
Contributor

@abnerh69 Please don't spam PRs for help. PRs are for discussing the specific PR, nothing else.

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.

4 participants