Skip to content

[pointer_interceptor] Add ios implementation and move web implementation to federated structure #5500

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

LouiseHsu
Copy link
Contributor

Addresses flutter/flutter#30143 by adding an iOS implementation

This PR is Part 2 of #5233

Pre-launch Checklist

  • 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 [relevant style guides] and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use dart format.)
  • I signed the [CLA].
  • The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the [pub versioning philosophy], or this PR is [exempt from version changes].
  • I updated CHANGELOG.md to add a description of the change, [following repository CHANGELOG style].
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • All existing and new tests are passing.

Copy link
Contributor

Choose a reason for hiding this comment

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

A trick we came up with while federating the other plugins that preserves git history better (allowing these to be treated as moves instead of copies) is to have this PR add a publish_to: none to pointer_interceptor/pointer_interceptor with a comment saying it's temporarily not being published while federation is in progress, then include deleting the web files from that package in this PR. Then the app-facing PR would remove that again as part of the changes to depend on this package.

Copy link
Contributor Author

@LouiseHsu LouiseHsu Nov 29, 2023

Choose a reason for hiding this comment

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

How do the ci tests pass for pointerinterceptor/pointerinterceptor in that case? or do I just gut the app facing package completely?

Copy link
Contributor

@stuartmorgan-g stuartmorgan-g Nov 29, 2023

Choose a reason for hiding this comment

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

Oh right, sorry. It's been a while since I did this last.

Looking back at my old PRs, what you would do is actually make all of the app-facing changes related to web in the same PR, and add path-based dependencies on the web package. Then tests will pass, and since it's not published it won't fail on having path-based deps.

Copy link
Contributor

Choose a reason for hiding this comment

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

If it's easier, you could include the iOS changes in the PR so you don't have to untangle them. If you do that, it would basically be what's left of the combo PR but with the temporary pubspec changes like the ones I linked to.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So making sure I understand you correctly: For this PR, I change the path dependencies in pointer_interceptor/pointer_interceptor and add publish_to: none, and also add any web/ios changes from that same folder?
and then for the next PR, it would just revert the pubspec changes + what ever is left?

Copy link
Contributor

@stuartmorgan-g stuartmorgan-g Nov 30, 2023

Choose a reason for hiding this comment

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

Yes, exactly. And the "whatever is left" part will probably be very minimal.

Sorry, if I'd remembered how I'd done this when moving existing implementations before I would have described the PR steps very differently; it's really more like 2.1 PRs (one for the platform interface, one for the rest with small temp hacks in the app-facing package, and then final cleanup to make the app-facing publishable again) than 3 in this case. What I told you initially about how to slice the PR into layers is the process for a more typical PR that's just adding something across the entire set of sub-packages.

@LouiseHsu LouiseHsu added the autosubmit Merge PR when tree becomes green via auto submit App label Dec 5, 2023
@auto-submit auto-submit bot merged commit 96310cc into flutter:main Dec 6, 2023
version: 0.10.0

environment:
sdk: '>=3.1.0 <=4.0.0'
Copy link
Contributor

Choose a reason for hiding this comment

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

I missed the <= here in review; it needs to be < since 4 would indicate breaking changes, so we don't know if it will work with 4. This is causing publish to fail, so I need to revert the PR.

version: 0.10.0

environment:
sdk: '>=3.1.0 <=4.0.0'
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here.

@stuartmorgan-g stuartmorgan-g added the revert Label used to revert changes in a closed and merged pull request. label Dec 6, 2023
auto-submit bot pushed a commit that referenced this pull request Dec 6, 2023
…lementation to federated structure (#5500)"

This reverts commit 96310cc.
@auto-submit auto-submit bot removed the revert Label used to revert changes in a closed and merged pull request. label Dec 6, 2023
auto-submit bot added a commit that referenced this pull request Dec 6, 2023
…plementation to federated structure" (#5591)

Reverts #5500
Initiated by: stuartmorgan
This change reverts the following previous change:
Original Description:
Addresses flutter/flutter#30143 by adding an iOS implementation 

This PR is Part 2 of #5233
@stuartmorgan-g
Copy link
Contributor

If you make a new PR with one commit that reverts f47b3b6 without changes and then a second commit that fixes the two constraints, I can review it ASAP.

@LouiseHsu LouiseHsu mentioned this pull request Dec 6, 2023
11 tasks
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 7, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Dec 7, 2023
flutter/packages@ca16173...15584a3

2023-12-07 [email protected] [gis_web] Migrate to package:web. (flutter/packages#5581)
2023-12-07 [email protected] Drop quiver usage in several packages (flutter/packages#5595)
2023-12-07 [email protected] [video_player] Unfork publish: for macOS (flutter/packages#5578)
2023-12-07 [email protected] [web_benchmarks] migrate to pkg:web (flutter/packages#5592)
2023-12-07 [email protected] [animations] Bump minimum supported Dart version to 3.2 (flutter/packages#5598)
2023-12-06 [email protected] [animations] Bump minimum Flutter version (flutter/packages#5596)
2023-12-06 [email protected] Migrate Material curves to new names (flutter/packages#4898)
2023-12-06 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[pointer_interceptor] Add ios implementation and move web implementation to federated structure" (flutter/packages#5591)
2023-12-06 [email protected] [pointer_interceptor] Add ios implementation and move web implementation to federated structure (flutter/packages#5500)
2023-12-06 [email protected] [pigeon] Use dart:io output inheritance for tooling (flutter/packages#5536)
2023-12-06 [email protected] Fix benchmark reload bug and migrate away from deprecated `js_util` APIs (flutter/packages#5577)
2023-12-06 [email protected] [google_sign_in] Add macOS support (flutter/packages#4962)
2023-12-06 [email protected] [rfw,flutter_markdown] Apparently you need a comma to end an //ignore (flutter/packages#5582)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
auto-submit bot pushed a commit that referenced this pull request Dec 7, 2023
This is essentially the same PR as #5500, with pubspec updates
Addresses flutter/flutter#30143 by adding an iOS implementation

This PR is Part 2 of #5233
HugoOlthof pushed a commit to moneybird/packages that referenced this pull request Dec 13, 2023
…ion to federated structure (flutter#5500)

Addresses flutter/flutter#30143 by adding an iOS implementation 

This PR is Part 2 of flutter#5233
HugoOlthof pushed a commit to moneybird/packages that referenced this pull request Dec 13, 2023
…plementation to federated structure" (flutter#5591)

Reverts flutter#5500
Initiated by: stuartmorgan
This change reverts the following previous change:
Original Description:
Addresses flutter/flutter#30143 by adding an iOS implementation 

This PR is Part 2 of flutter#5233
HugoOlthof pushed a commit to moneybird/packages that referenced this pull request Dec 13, 2023
This is essentially the same PR as flutter#5500, with pubspec updates
Addresses flutter/flutter#30143 by adding an iOS implementation

This PR is Part 2 of flutter#5233
@reidbaker reidbaker mentioned this pull request Jan 12, 2024
arc-yong pushed a commit to Arctuition/packages-arc that referenced this pull request Jun 14, 2024
…ion to federated structure (flutter#5500)

Addresses flutter/flutter#30143 by adding an iOS implementation 

This PR is Part 2 of flutter#5233
arc-yong pushed a commit to Arctuition/packages-arc that referenced this pull request Jun 14, 2024
…plementation to federated structure" (flutter#5591)

Reverts flutter#5500
Initiated by: stuartmorgan
This change reverts the following previous change:
Original Description:
Addresses flutter/flutter#30143 by adding an iOS implementation 

This PR is Part 2 of flutter#5233
arc-yong pushed a commit to Arctuition/packages-arc that referenced this pull request Jun 14, 2024
This is essentially the same PR as flutter#5500, with pubspec updates
Addresses flutter/flutter#30143 by adding an iOS implementation

This PR is Part 2 of flutter#5233
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App p: pointer_interceptor platform-ios platform-web
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants