-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Background Transparent feature for CupertinoSliverNavigationBar #142439
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
Background Transparent feature for CupertinoSliverNavigationBar #142439
Conversation
4893f4c to
f523ef0
Compare
5dcaaa5 to
d113f4d
Compare
MitchellGoodwin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for putting this together! This looks great so far, and already looks much closer to native. I'm wondering if there is room to make the transition slightly smoother.
| CupertinoDynamicColor.resolve(expandedBackgroundColor, context), | ||
| CupertinoDynamicColor.resolve(backgroundColor, context), | ||
| expandedState, | ||
| )! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My eyes may be playing tricks on me, but it looks like the background transition on the iPhone is a bit smoother than the one made by this PR. It also looks like the border may be on a different animation curve. It comes in somewhat suddenly at the end, when meanwhile the color had already started it's transformation. I wonder if their respective lerps should be looking at different double values? Maybe the one for color starts earlier? Or maybe a curve can be applied to it to start appearing sooner?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Mitchell, thanks for your feedback.
I've updated the expandedState calculations, it should be much smoother now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Demo:
2024-02-13.03.46.50.mov
|
Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change). If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
b8508fb to
41940a2
Compare
…er_cupertino_app_bar_transparent
…er_cupertino_app_bar_transparent
|
Golden file changes are available for triage from new commit, Click here to view. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
…er_cupertino_app_bar_transparent
…er_cupertino_app_bar_transparent
|
Golden file changes are available for triage from new commit, Click here to view. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this PR @meg4cyberc4t , I'm also very interested in getting this merged.
During my tests I found a way to make it look closer to native, (as suggested in the code comment).
The problem in the demo example you posted is that the space just below large title is empty so we don't see the blurring effect due to fully transparent background, or the absence of a border during the transition, which doesn't happen in native.
I made a demo with my own app, using the PR code, to show this:
Enregistrement.de.l.ecran.2024-04-25.a.10.31.24.mov
And here is the updated demo using the code change I suggest. It both fixes the blurry background and the absence of border during transition. The transition looks overall much closer to native now.
Enregistrement.de.l.ecran.2024-04-25.a.10.29.32.mov
| @override | ||
| Widget build(BuildContext context, double shrinkOffset, bool overlapsContent) { | ||
| final bool showLargeTitle = shrinkOffset < maxExtent - minExtent - _kNavBarShowLargeTitleThreshold; | ||
| final double expandedState = shrinkOffset > minExtent ? clampDouble((shrinkOffset - minExtent) / (maxExtent - minExtent) * 2, 0, 1) : 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd suggest a different calculation for expandedState so it looks closer to native:
final double largeTitleThreshold = maxExtent - minExtent - _kNavBarShowLargeTitleThreshold;
final bool showLargeTitle = shrinkOffset < largeTitleThreshold;
final double expandedState = shrinkOffset > largeTitleThreshold + _kNavBarShowLargeTitleThreshold
? clampDouble((shrinkOffset - largeTitleThreshold) / largeTitleThreshold, 0, 1)
: 0;|
@veloce Hi. Thank you for giving a good feedback with clear examples. I will work on this PR and come back in the future with a more "native-looking" implementation. |
|
I've been experimenting with it myself, and I got a POC that looks good in my app. I also modified the Code is not ready for a PR yet but here it is: 8280cba |
|
Hey @meg4cyberc4t do you still want to move forward with this PR? Or are the changes from @veloce something we should look at in a new PR? |
Hi. I think the changes from @veloce are more suited to our goal of making the widget "more native". This PR can be closed. |
|
Changes from the above comment are outdated now, there were issues with route transitions. It works well but not yet ready for a PR. I'll try to make one, but I'm not sure about the API. In this patch I don't use anymore the transparency (because of route transition issues and unwanted blurring effects), and rely instead on the But I think if we're going to change the top nav bar, we should also take care of the bottom tab bar. |
I think it'd be fine if there was an optional parameter, as long as it's able to intelligently get a default. So if it's wrapped by a And does the tab bar act similarly on iOS? Or are you referring to other updates? |
Yes I could see it in several iOS apps (files, notes, ...), the bottom tab bar is transparent when no content is scrolled under. |
Then I can try to make a PR with that behaviour. |
|
Then if the bottom tab bar does it as well, we would want that in the Cupertino widget as well. If you're willing to include it in your PR, that'd be appreciated, assuming it uses the same logic as the top bar, but in reverse. If it needs a different implementation it would probably be better if it's in its own PR. If you just want to work on the top bar, I can file a new issue for the tab bar. |
|
I will try make the PR with both top and bottom bar next week. |
This PR is making the `CupertinoNavigationBar` and `CupertinoSliverNavigationBar` appear transparent as long as the content is not scrolled under them, so they look like standard iOS apps nav bars. https://github.com/flutter/flutter/assets/423393/eee2700b-2a91-4577-922c-6163d47cb357 https://github.com/flutter/flutter/assets/423393/3847f2b5-0dac-4d5e-aa6f-03c1d2893e30 <details> <summary>Demo app code</summary> ```dart import 'package:flutter/cupertino.dart'; /// Flutter code sample for [CupertinoTabScaffold]. void main() => runApp(const TabScaffoldApp()); class TabScaffoldApp extends StatefulWidget { const TabScaffoldApp({super.key}); @OverRide State<TabScaffoldApp> createState() => _TabScaffoldAppState(); } class _TabScaffoldAppState extends State<TabScaffoldApp> { Brightness _brightness = Brightness.light; @OverRide Widget build(BuildContext context) { return CupertinoApp( theme: CupertinoThemeData(brightness: _brightness), home: TabScaffoldExample( brightness: _brightness, onBrightnessToggle: _toggleBrightness), ); } void _toggleBrightness() { setState(() { _brightness = _brightness == Brightness.light ? Brightness.dark : Brightness.light; }); } } class TabScaffoldExample extends StatefulWidget { const TabScaffoldExample( {required this.brightness, required this.onBrightnessToggle, super.key}); final VoidCallback onBrightnessToggle; final Brightness brightness; @OverRide State<TabScaffoldExample> createState() => _TabScaffoldExampleState(); } class _TabScaffoldExampleState extends State<TabScaffoldExample> { @OverRide Widget build(BuildContext context) { return CupertinoTabScaffold( tabBar: CupertinoTabBar( items: const <BottomNavigationBarItem>[ BottomNavigationBarItem( icon: Icon(CupertinoIcons.home), label: 'Home', ), BottomNavigationBarItem( icon: Icon(CupertinoIcons.search_circle_fill), label: 'Explore', ), BottomNavigationBarItem( icon: Icon(CupertinoIcons.person_fill), label: 'Profile', ), BottomNavigationBarItem( icon: Icon(CupertinoIcons.settings_solid), label: 'Settings', ), ], ), tabBuilder: (BuildContext context, int index) { return CupertinoTabView( builder: (BuildContext context) { return CupertinoPageScaffold( backgroundColor: index == 3 ? CupertinoColors.secondarySystemBackground .resolveFrom(context) : null, child: CustomScrollView( slivers: [ CupertinoSliverNavigationBar( largeTitle: Text('Tab $index'), initiallyTransparent: index != 2, trailing: CupertinoButton( padding: EdgeInsets.zero, onPressed: widget.onBrightnessToggle, child: Icon( widget.brightness == Brightness.light ? CupertinoIcons.moon_stars : CupertinoIcons.sun_max, ), ), ), SliverSafeArea( top: false, sliver: SliverList.list( children: [ CupertinoButton( child: const Text('Next page'), onPressed: () { Navigator.of(context).push( CupertinoPageRoute<void>( builder: (BuildContext context) { return CupertinoPageScaffold( navigationBar: CupertinoNavigationBar( middle: Text('Inner page of tab $index'), ), child: ListView( children: [ Center( child: CupertinoButton( child: const Text('Back'), onPressed: () { Navigator.of(context).pop(); }, ), ), if (index == 0) const _LongList(), const SizedBox(height: 20), ], ), ); }, ), ); }, ), if (index == 1) const _LongList(), const SizedBox(height: 20), ], ), ), ], ), ); }, ); }, ); } } class _LongList extends StatelessWidget { const _LongList(); @OverRide Widget build(BuildContext context) { return Column( children: [ for (int i = 0; i < 50; i++) ...[ CupertinoListTile( leading: const Icon(CupertinoIcons.book), title: Text('Bookstore item $i'), ), ], ], ); } } ``` </details> This is the continuation of #142439. I tried to keep the simplest API possible, so it's only introducing a new `automaticBackgroundVisibility` boolean parameter. In the implementation I had to use the `CupertinoPageScaffold` background color to make it look transparent instead of a 0 opacity, because of issues with route transitions. I used an `InheritedWidget` so the nav bar is always getting the right background color from the parent scaffold, whether it is overridden or not. It would probably be better to make the inherited widget private but we'd need to move all the nav bar code to the same file as the scaffold, so for now I've just hidden it from the export. Let me know if it is okay to do that. This PR is not dealing with the bottom tab bar, because the same [straightforward approach](veloce@dde8ec6) doesn't work here. The problem is that the scroll notification is sent only when the scroll view is created or updated, so it doesn't work if one pushes a screen and navigates back. Issues: - #78607 - #60411
This PR is making the `CupertinoNavigationBar` and `CupertinoSliverNavigationBar` appear transparent as long as the content is not scrolled under them, so they look like standard iOS apps nav bars. https://github.com/flutter/flutter/assets/423393/eee2700b-2a91-4577-922c-6163d47cb357 https://github.com/flutter/flutter/assets/423393/3847f2b5-0dac-4d5e-aa6f-03c1d2893e30 <details> <summary>Demo app code</summary> ```dart import 'package:flutter/cupertino.dart'; /// Flutter code sample for [CupertinoTabScaffold]. void main() => runApp(const TabScaffoldApp()); class TabScaffoldApp extends StatefulWidget { const TabScaffoldApp({super.key}); @OverRide State<TabScaffoldApp> createState() => _TabScaffoldAppState(); } class _TabScaffoldAppState extends State<TabScaffoldApp> { Brightness _brightness = Brightness.light; @OverRide Widget build(BuildContext context) { return CupertinoApp( theme: CupertinoThemeData(brightness: _brightness), home: TabScaffoldExample( brightness: _brightness, onBrightnessToggle: _toggleBrightness), ); } void _toggleBrightness() { setState(() { _brightness = _brightness == Brightness.light ? Brightness.dark : Brightness.light; }); } } class TabScaffoldExample extends StatefulWidget { const TabScaffoldExample( {required this.brightness, required this.onBrightnessToggle, super.key}); final VoidCallback onBrightnessToggle; final Brightness brightness; @OverRide State<TabScaffoldExample> createState() => _TabScaffoldExampleState(); } class _TabScaffoldExampleState extends State<TabScaffoldExample> { @OverRide Widget build(BuildContext context) { return CupertinoTabScaffold( tabBar: CupertinoTabBar( items: const <BottomNavigationBarItem>[ BottomNavigationBarItem( icon: Icon(CupertinoIcons.home), label: 'Home', ), BottomNavigationBarItem( icon: Icon(CupertinoIcons.search_circle_fill), label: 'Explore', ), BottomNavigationBarItem( icon: Icon(CupertinoIcons.person_fill), label: 'Profile', ), BottomNavigationBarItem( icon: Icon(CupertinoIcons.settings_solid), label: 'Settings', ), ], ), tabBuilder: (BuildContext context, int index) { return CupertinoTabView( builder: (BuildContext context) { return CupertinoPageScaffold( backgroundColor: index == 3 ? CupertinoColors.secondarySystemBackground .resolveFrom(context) : null, child: CustomScrollView( slivers: [ CupertinoSliverNavigationBar( largeTitle: Text('Tab $index'), initiallyTransparent: index != 2, trailing: CupertinoButton( padding: EdgeInsets.zero, onPressed: widget.onBrightnessToggle, child: Icon( widget.brightness == Brightness.light ? CupertinoIcons.moon_stars : CupertinoIcons.sun_max, ), ), ), SliverSafeArea( top: false, sliver: SliverList.list( children: [ CupertinoButton( child: const Text('Next page'), onPressed: () { Navigator.of(context).push( CupertinoPageRoute<void>( builder: (BuildContext context) { return CupertinoPageScaffold( navigationBar: CupertinoNavigationBar( middle: Text('Inner page of tab $index'), ), child: ListView( children: [ Center( child: CupertinoButton( child: const Text('Back'), onPressed: () { Navigator.of(context).pop(); }, ), ), if (index == 0) const _LongList(), const SizedBox(height: 20), ], ), ); }, ), ); }, ), if (index == 1) const _LongList(), const SizedBox(height: 20), ], ), ), ], ), ); }, ); }, ); } } class _LongList extends StatelessWidget { const _LongList(); @OverRide Widget build(BuildContext context) { return Column( children: [ for (int i = 0; i < 50; i++) ...[ CupertinoListTile( leading: const Icon(CupertinoIcons.book), title: Text('Bookstore item $i'), ), ], ], ); } } ``` </details> This is the continuation of flutter#142439. I tried to keep the simplest API possible, so it's only introducing a new `automaticBackgroundVisibility` boolean parameter. In the implementation I had to use the `CupertinoPageScaffold` background color to make it look transparent instead of a 0 opacity, because of issues with route transitions. I used an `InheritedWidget` so the nav bar is always getting the right background color from the parent scaffold, whether it is overridden or not. It would probably be better to make the inherited widget private but we'd need to move all the nav bar code to the same file as the scaffold, so for now I've just hidden it from the export. Let me know if it is okay to do that. This PR is not dealing with the bottom tab bar, because the same [straightforward approach](veloce@dde8ec6) doesn't work here. The problem is that the scroll notification is sent only when the scroll view is created or updated, so it doesn't work if one pushes a screen and navigates back. Issues: - flutter#78607 - flutter#60411
Previously, the behavior of the CupertinoSliverNavigationBar when
2024-01-29.13.52.50.mov
it was different from the native one:
2024-01-29.13.55.43.mov
As you can see, there is no way to make the background transparent in the expanded state if a largeTitle is specified.
In this PR, expandedTransparent is added to CupertinoSliverNavigationBar, which makes such a more native functionality.
2024-01-29.14.01.02.mov
Issues:
#78607
#60411
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.