Dark mode for CupertinoSwitch and CupertinoScrollbar, Fidelity updates #40636

LongCatIsLooong · 2019-09-17T00:20:18Z

Description

Dark mode and fidelity updates. This change breaks goldens and the API of CupertinoThumbPainter.

Screenshots

Comparison

Native UIKit	Cupertino

Dark Mode

activeOrange will be replaced once CupertinoTheme is updated. For now please bear with the wrong slider track color.

switches	sliders

Related Issues

part of #35541
Fixes #37312

Tests

I added the following tests:

Switch renders correctly in dark mode
Scrollbar dark mode

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.

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I signed the CLA.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I updated/added relevant documentation (doc comments with ///).
All existing and new tests are passing.
The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require Flutter developers to manually update their apps to accommodate your change?

Yes, this is a breaking change (Please read Handling breaking changes). Replace this with a link to the e-mail where you asked for input on this proposed change.
No, this is not a breaking change.

…part1

fluttergithubbot · 2019-09-17T00:20:21Z

It looks like this pull request includes a golden file change. Please make sure to follow Handling Breaking Changes. While there are exceptions to this rule, if this patch modifies an existing golden file, it is probably not an exception. Only new golden files are not considered breaking changes.

Writing a golden file test for package:flutter may also provide guidance for this change.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

LongCatIsLooong · 2019-09-17T02:34:33Z

bin/internal/goldens.version

@@ -1 +1 @@
-ae7615ce466a548b41a0f7b9860ec453646f73e9
+aec919aee765d460a3dbe457e81c29e0a2221334


This hash is a placeholder.

LongCatIsLooong · 2019-09-17T17:06:50Z

packages/flutter/lib/src/cupertino/switch.dart

  }

-  final CupertinoThumbPainter _thumbPainter = CupertinoThumbPainter();
+  final CupertinoThumbPainter _thumbPainter = const CupertinoThumbPainter.switchThumb();


Interestingly the analyzer doesn't seem to have an opinion about the const. I assume const CupertinoThumbPainter.switchThumb() is different from CupertinoThumbPainter.switchThumb() and the former should be preferred?

Weird, I'm curious as to why. Definitely better to err on the side of including the const though.

Why not just substitute const CupertinoThumbPainter.switchThumb(); for _thumbPainter below?

Piinks · 2019-09-17T17:10:25Z

packages/flutter/test/cupertino/scrollbar_test.dart


 import '../rendering/mock_canvas.dart';

+


Nit: Extra blank line

justinmc

LGTM

Have you seen #37312 and does this fix that as well?

justinmc · 2019-09-17T18:18:15Z

packages/flutter/lib/src/cupertino/switch.dart

  }

-  final CupertinoThumbPainter _thumbPainter = CupertinoThumbPainter();
+  final CupertinoThumbPainter _thumbPainter = const CupertinoThumbPainter.switchThumb();


Weird, I'm curious as to why. Definitely better to err on the side of including the const though.

LongCatIsLooong · 2019-09-17T19:50:27Z

@justinmc I believe it does: flutter/goldens#45. Updated the description to include that issue. Thanks for bringing it up! 👍

…part1

HansMuller

LGTM with a few small changes

HansMuller · 2019-09-18T00:07:23Z

packages/flutter/lib/src/cupertino/scrollbar.dart

+// Extracted from iOS 13.1 beta using Debug View Hierarchy.
+const Color _kScrollbarColor = CupertinoDynamicColor.withBrightness(
+  color: Color(0x59000000),
+  darkColor:Color(0x80FFFFFF),


space after the colon

HansMuller · 2019-09-18T00:14:26Z

packages/flutter/lib/src/cupertino/scrollbar.dart

+    _painter ??= _buildCupertinoScrollbarPainter(context);
+    _painter
+      ..textDirection = Directionality.of(context)
+      ..color = CupertinoDynamicColor.resolve(_kScrollbarColor, context);


Need to update the painter's padding too, since it depends on MediaQuery.of(context).

HansMuller · 2019-09-18T00:14:53Z

packages/flutter/lib/src/cupertino/slider.dart


    final Offset thumbCenter = Offset(trackActive, trackCenter);
-    _thumbPainter.paint(canvas, Rect.fromCircle(center: thumbCenter, radius: CupertinoThumbPainter.radius));
+    const CupertinoThumbPainter().paint(canvas, Rect.fromCircle(center: thumbCenter, radius: CupertinoThumbPainter.radius));


HansMuller · 2019-09-18T00:18:43Z

packages/flutter/lib/src/cupertino/switch.dart

  }

-  final CupertinoThumbPainter _thumbPainter = CupertinoThumbPainter();
+  final CupertinoThumbPainter _thumbPainter = const CupertinoThumbPainter.switchThumb();


Why not just substitute const CupertinoThumbPainter.switchThumb(); for _thumbPainter below?

HansMuller · 2019-09-18T00:23:35Z

packages/flutter/lib/src/widgets/scrollbar.dart

+  set textDirection(TextDirection value) {
+    assert(value != null);
+    if (textDirection == value)
+    return;


two space indent

brandondiamond

LGTM!

…part1

flutter#40636)

LongCatIsLooong added 9 commits September 13, 2019 15:22

scroll bar

d78da69

update

9302ef3

switch dark mode

87c15f2

update

4bcb600

Merge branch 'master' of github.com:flutter/flutter into dark-mode-s-…

fbbfd2d

…part1

checkout master segmented control

365c8bf

update thumb painter

a61777e

with fake golden hash

1bc8fe0

Merge branch 'master' of github.com:flutter/flutter into dark-mode-s-…

876713b

…part1

LongCatIsLooong requested a review from Piinks as a code owner September 17, 2019 00:20

googlebot added the cla: yes label Sep 17, 2019

LongCatIsLooong mentioned this pull request Sep 17, 2019

Goldens for https://github.com/flutter/flutter/pull/40636 flutter/goldens#45

Merged

LongCatIsLooong added the work in progress; do not review label Sep 17, 2019

shadow offset

b4b3e5a

LongCatIsLooong removed the work in progress; do not review label Sep 17, 2019

LongCatIsLooong changed the title ~~Dark mode for CupertinoSwitch and CupertinoScrollbar~~ Dark mode for CupertinoSwitch and CupertinoScrollbar, Fidelity updates Sep 17, 2019

LongCatIsLooong commented Sep 17, 2019

View reviewed changes

LongCatIsLooong requested review from HansMuller, darrenaustin and justinmc September 17, 2019 17:03

LongCatIsLooong commented Sep 17, 2019

View reviewed changes

Piinks reviewed Sep 17, 2019

View reviewed changes

review

6a93dd5

LongCatIsLooong force-pushed the dark-mode-s-part1 branch from 5608e22 to 6a93dd5 Compare September 17, 2019 18:04

update docs

ba6b7f4

justinmc approved these changes Sep 17, 2019

View reviewed changes

Merge branch 'master' of github.com:flutter/flutter into dark-mode-s-…

0f8545a

…part1

LongCatIsLooong requested a review from brandondiamond September 17, 2019 21:49

HansMuller approved these changes Sep 18, 2019

View reviewed changes

LongCatIsLooong added 2 commits September 18, 2019 12:42

review

3e4218c

remove unnecessary import

4dc5f9e

brandondiamond approved these changes Sep 18, 2019

View reviewed changes

LongCatIsLooong added 2 commits September 23, 2019 10:16

update border

8b1a403

Merge branch 'master' of github.com:flutter/flutter into dark-mode-s-…

c7d9d8a

…part1

LongCatIsLooong added a commit to flutter/goldens that referenced this pull request Sep 23, 2019

Goldens for flutter/flutter#40636 (#45)

0728c1b

update golden hash

8dad9c4

LongCatIsLooong merged commit 4815b26 into flutter:master Sep 24, 2019

LongCatIsLooong deleted the dark-mode-s-part1 branch September 24, 2019 07:21

Inconnu08 pushed a commit to Inconnu08/flutter that referenced this pull request Sep 30, 2019

Dark mode for CupertinoSwitch and CupertinoScrollbar, Fidelity updates (

8c9e710

flutter#40636)

HansMuller mentioned this pull request Oct 18, 2019

Upcoming Cupertino Library Features #43041

Closed

18 tasks

github-actions bot locked as resolved and limited conversation to collaborators Aug 4, 2021

		@@ -1 +1 @@
		ae7615ce466a548b41a0f7b9860ec453646f73e9
		aec919aee765d460a3dbe457e81c29e0a2221334

Dark mode for CupertinoSwitch and CupertinoScrollbar, Fidelity updates #40636

Dark mode for CupertinoSwitch and CupertinoScrollbar, Fidelity updates #40636

Uh oh!

Conversation

LongCatIsLooong commented Sep 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Screenshots

Comparison

Dark Mode

Related Issues

Tests

Checklist

Breaking Change

Uh oh!

fluttergithubbot commented Sep 17, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LongCatIsLooong Sep 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LongCatIsLooong commented Sep 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

HansMuller left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brandondiamond left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

LongCatIsLooong commented Sep 17, 2019 •

edited

Loading

LongCatIsLooong Sep 17, 2019 •

edited

Loading

LongCatIsLooong commented Sep 17, 2019 •

edited

Loading