Update cupertino demos in gallery #43841
Conversation
LongCatIsLooong
force-pushed
the
update-gallery
branch
from
2d93082 to
15f0dd4
Compare
LongCatIsLooong
force-pushed
the
update-gallery
branch
from
531343b to
b4e530c
Compare
LongCatIsLooong
changed the title
|
Is the black on blue contrast for the cupertino button going to get any better with CupertinoDynamicColor (or is it already using it?). The contrast looks a bit low unless the blue is going to look a bit brighter with dynamic colors. |
|
@xster thank you for checking! Almost all Cupertino widgets are already using dynamic colors, including |
justinmc
left a comment
justinmc
left a comment
There was a problem hiding this comment.
LGTM
The screenshots look really good. Thanks for doing all the tedious work here!
| scrollController: scrollController, | ||
| itemExtent: _kPickerItemHeight, | ||
| backgroundColor: CupertinoColors.white, | ||
| backgroundColor: CupertinoDynamicColor.resolve(CupertinoColors.systemBackground, context), |
There was a problem hiding this comment.
Should this be
CupertinoColors.systemBackground.resolveFrom(context)Here and elsewhere.
There was a problem hiding this comment.
Currently the nullOk parameter of CupertinoDynamicColor.resolveFrom defaults to false. I think I should change it to true?
| ); | ||
| } | ||
|
|
||
| Widget _buildBottomPicker(Widget picker) { |
There was a problem hiding this comment.
I think it would be more Fluttery (might be more efficient too) if we defined a _BottomPicker widget instead of using this function. Doing so would also eliminate the need for the Builder.
Likewise for the other _buildFoo() functions.
Note also: since the demo was already factored this way; it would be OK to make the change in a follow-on PR.
| @override | ||
| void initState() { | ||
| super.initState(); | ||
| controller.addListener(() { setState(() {}); }); |
There was a problem hiding this comment.
I don't think we want to require developers to set up the segmented control this way. The convention for this kind of thing is to for the onValueChanged() callback set update the enclosing widget's state and call setState(). Conventionally, the controller parameter is optional. If the controller is specified then the caller must manage its lifetime.
|
|
LongCatIsLooong
force-pushed
the
update-gallery
branch
from
e656429 to
19d0fd5
Compare
6 participants
Description
Can't directly use
CupertinoDynamicColorinCupertinoPickers because the relevant PR is reverted and has to re-land first.Related Issues
#35541
Fixes #39685
Tests
I added the following tests:
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Does your PR require Flutter developers to manually update their apps to accommodate your change?