Skip to content

Use now-upstream ScrollView.paintOrder, replacing CustomPaintOrderScrollView #1489

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
Apr 30, 2025

Conversation

gnprice
Copy link
Member

@gnprice gnprice commented Apr 28, 2025

Commit messages

47273ce deps: Upgrade Flutter to 3.32.0-1.0.pre.257

And update Flutter's supporting libraries to match.

In particular this pulls in two recent PRs of mine so we can
use them:
flutter/flutter#166730
flutter/flutter#164818

7b5ed74 scroll [nfc]: Use now-upstream ScrollView.paintOrder

The functionality provided by CustomPaintOrderScrollView is now
available upstream, since my PR adding it landed:
flutter/flutter#164818

705db8c scroll [nfc]: Cut CustomPaintOrderScrollView now that upstream PR merged

This functionality is now covered by [ScrollView.paintOrder],
after my PR adding that feature upstream.

gnprice added 3 commits April 28, 2025 16:27
And update Flutter's supporting libraries to match.

In particular this pulls in two recent PRs of mine so we can
use them:
  flutter/flutter#166730
  flutter/flutter#164818
The functionality provided by CustomPaintOrderScrollView is now
available upstream, since my PR adding it landed:
  flutter/flutter#164818
This functionality is now covered by [ScrollView.paintOrder],
after my PR adding that feature upstream.
@gnprice gnprice added the maintainer review PR ready for review by Zulip maintainers label Apr 28, 2025
@chrisbobbe chrisbobbe merged commit 7ca1ff7 into zulip:main Apr 30, 2025
1 check passed
@chrisbobbe
Copy link
Collaborator

Thanks, LGTM! Merged.

@gnprice gnprice deleted the pr-paintorder-upstream branch April 30, 2025 21:38
@chrisbobbe
Copy link
Collaborator

Oops, not having run this locally, I missed something: the new minimum Flutter version in the Flutter upgrade commit—

deps: Upgrade Flutter to 3.32.0-1.0.pre.257

—contains flutter/flutter@09d4dab, which causes a change in an iOS file when I run flutter run on my Mac:

diff --git ios/Flutter/AppFrameworkInfo.plist ios/Flutter/AppFrameworkInfo.plist
index 7c5696400..1dc6cf765 100644
--- ios/Flutter/AppFrameworkInfo.plist
+++ ios/Flutter/AppFrameworkInfo.plist
@@ -21,6 +21,6 @@
   <key>CFBundleVersion</key>
   <string>1.0</string>
   <key>MinimumOSVersion</key>
-  <string>12.0</string>
+  <string>13.0</string>
 </dict>
 </plist>

@gnprice
Copy link
Member Author

gnprice commented Apr 30, 2025

Ah good catch, thanks. Would you send a quick PR to update that?

This is an example of where it'd be handy to have #773, building for iOS in our CI.

@gnprice
Copy link
Member Author

gnprice commented Apr 30, 2025

In fact I guess our minimum is already iOS 14 in zulip-mobile, as I found when checking here:
#mobile > platform versions @ 💬

So we should probably increase it further to there.

chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this pull request May 1, 2025
The Flutter doc on deploying an iOS app says this number should
match the deployment target we set in Xcode (which we also specify
in ios/Podfile):
  https://docs.flutter.dev/deployment/ios#updating-the-apps-deployment-version
; sure, go ahead and update it.

Prompted by noticing that `flutter run` started bumping it to 13.0
(in flutter/flutter@09d4dabd6; see discussion:
  zulip#1489 (comment)
) and realizing that 14.0 is actually what it should be for us.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintainer review PR ready for review by Zulip maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants