Re-lands platform brightness support on iOS #10791

matthew-carroll · 2019-08-09T00:22:06Z

Re-lands platform brightness support on iOS with an #ifdef for CI.

chinmaygarde

This doesn't compile on ToT. Have you tried switching to the release Xcode (sudo xcode-select -s /path/to/non/beta/xcode.app) and trying a build? I think you meant #else instead of #elseif

The error:

/Users/chinmaygarde/goma/gomacc  ../../buildtools/mac-x64/clang/bin/clang++ -MMD -MF obj/flutter/shell/platform/darwin/ios/framework/Source/libFlutter.FlutterViewController.o.d -DFLUTTER_FRAMEWORK=1 -DNO_TCMALLOC -DMEMORY_TOOL_REPLACES_ALLOCATOR -DMEMORY_SANITIZER_INITIAL_SIZE -DTOOLCHAIN_VERSION=bac220c15490dcf7b7d8136f75100bbc77e8d217 -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D_FORTIFY_SOURCE=2 -D_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS -D_LIBCPP_ENABLE_THREAD_SAFETY_ANNOTATIONS -D_DEBUG -DSK_GL -DSK_ENABLE_DUMP_GPU -DSK_HAS_JPEG_LIBRARY -DSK_HAS_PNG_LIBRARY -DSK_HAS_WEBP_LIBRARY -DSK_HAS_WUFFS_LIBRARY -DFLUTTER_RUNTIME_MODE_DEBUG=1 -DFLUTTER_RUNTIME_MODE_PROFILE=2 -DFLUTTER_RUNTIME_MODE_RELEASE=3 -DFLUTTER_RUNTIME_MODE=1 -DU_USING_ICU_NAMESPACE=0 -DU_ENABLE_DYLOAD=0 -DU_STATIC_IMPLEMENTATION -DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_FILE -DUCHAR_TYPE=uint16_t -DSK_DISABLE_REDUCE_OPLIST_SPLITTING -DSK_LEGACY_SKCODEC_NONE_ENUM -DSK_GL -DSK_ENABLE_DUMP_GPU -DSK_DISABLE_AAA -DSK_DISABLE_READBUFFER -DSK_DISABLE_EFFECT_DESERIALIZATION -DSK_DISABLE_LEGACY_SHADERCONTEXT -DSK_DISABLE_LOWP_RASTER_PIPELINE -DSK_FORCE_RASTER_PIPELINE_BLITTER -DSK_ASSUME_GL_ES=1 -DRAPIDJSON_HAS_STDSTRING -DRAPIDJSON_HAS_CXX11_RANGE_FOR -DRAPIDJSON_HAS_CXX11_RVALUE_REFS -DRAPIDJSON_HAS_CXX11_TYPETRAITS -DRAPIDJSON_HAS_CXX11_NOEXCEPT -I../.. -Igen -I../.. -I../../flutter/third_party/txt/src -I../../third_party/harfbuzz/src -I../../third_party/icu/source/common -I../../third_party/icu/source/i18n -I../../third_party/skia -I../../third_party/rapidjson -I../../third_party/rapidjson/include -I../../third_party/dart/runtime -I../../third_party/dart/runtime/include -I../../third_party -I../../flutter/shell/platform/darwin/common/framework/Headers -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS12.4.sdk -miphoneos-version-min=8.0   -fno-strict-aliasing -arch arm64 -fcolor-diagnostics -Wall -Wextra -Wendif-labels -Werror -Wno-missing-field-initializers -Wno-unused-parameter -Wunguarded-availability -fvisibility=hidden -stdlib=libc++ -Wheader-hygiene -Wstring-conversion -Wthread-safety -O0 -g2  -fvisibility-inlines-hidden -fobjc-call-cxx-cdtors -std=c++17 -fno-rtti -fno-exceptions  -c ../../flutter/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm -o obj/flutter/shell/platform/darwin/ios/framework/Source/libFlutter.FlutterViewController.o
../../flutter/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm:973:6: error: expected value in expression
#elif
     ^
1 error generated.

matthew-carroll · 2019-08-09T00:31:54Z

Weird that this seemed to compile for me with the wrong syntax...I'll switch to #else

stuartmorgan-g · 2019-08-09T00:32:43Z

shell/platform/darwin/ios/framework/Source/FlutterViewController.mm

+- (NSString*)brightnessMode {
+#if defined(__IPHONE_13_0)
+  if (@available(iOS 13, *)) {
+    UIUserInterfaceStyle style = UITraitCollection.currentTraitCollection.userInterfaceStyle;


Why aren't you just using self.traitCollection here? That would avoid the SDK problem, right?

I'll give that a try

matthew-carroll · 2019-08-09T00:39:04Z

@chinmaygarde @stuartmorgan I switched to the VC's traitCollection and removed the API check

chinmaygarde · 2019-08-09T00:57:18Z

shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest.m

+  id settingsChannel = OCMClassMock([FlutterBasicMessageChannel class]);
+  OCMStub([engine settingsChannel]).andReturn(settingsChannel);
+
+  FlutterViewController* vc = [[FlutterViewController alloc] initWithEngine:engine


Are these tests running with ARC? Because these look like leaks. The rest of the engine does not use ARC.

No idea. I was just continuing with what I already found in this file. If you have any specific recommendations for changes, I can make them, but I'm working from a lack of iOS knowledge here.

@chinmaygarde @stuartmorgan do either of you have any specific instructions for what should be done here?

@dnfield @gspencergoog do either of you know what changes should be made here to avoid leaks? I don't know what changes to make to bring this test code into compliance...

I would turn on ARC for the test file, since that would be easy (it looks like the test project is doing it file-by-file, which seems like an odd approach; I would just enable it for the whole project).

Failing that, add an autorelease here.

@LongCatIsLooong helped me get ARC enabled

We may want to have ARC off for the project so that we can explicitly test deallocating things...

I believe I only enabled it for this file. Do you want me to change something?

No, that's fine - my reply was more directed towards @stuartmorgan's comment :)

Well since the only test in that file is leaking the view controller, I'm pretty sure it's not deliberately off to test dealloc...

chinmaygarde · 2019-08-09T00:58:43Z

testing/ios/IosUnitTests/run_tests.sh

@@ -12,6 +12,6 @@ fi

 set -o pipefail && xcodebuild -sdk iphonesimulator \
  -scheme IosUnitTests \
-  -destination 'platform=iOS Simulator,name=iPhone SE,OS=12.2' \
+  -destination 'platform=iOS Simulator,name=iPhone 8,OS=13.0' \


This simulator variant won't be available on ToT.

Reverted the OS version, but kept it on iPhone 8 because my local environment doesn't even offer an iPhone SE

FWIW you should be able to add iPhone SE locally, but AFAIK know of our tests depend on the simulator type at this point.

stuartmorgan-g · 2019-08-09T02:46:00Z

shell/platform/darwin/ios/framework/Source/FlutterViewController.mm

@@ -690,6 +695,11 @@ - (CGFloat)statusBarPadding {
  return CGRectIsNull(intersection) ? 0.0 : intersection.size.height;
 }

+- (void)viewWillLayoutSubviews {
+  [super viewWillLayoutSubviews];
+  [self onUserSettingsChanged:nil];


Looking more broadly at the patch rather than just the part that was failing before: why are all these layout-based methods getting onUserSettingChanged: calls? traitCollectionDidChange: makes sense, but I don't understand why a relayout would change any settings (and especially why each relayout would need to update the settings 3 times, as this change appears to be doing.)

Here's the guide I was following:
https://developer.apple.com/documentation/appkit/supporting_dark_mode_in_your_interface?language=objc

That guide says that all four of those methods are called on appearance changes, and that appearance-sensitive code should be moved into one of them. That's very different than saying that you should re-run all appearance-sensitive code in all of them.

Alright. Do you have a recommendation? That guide does not disambiguate execution paths that lead to those methods, so it's not clear to me which subset of those methods will actually account for all use-cases where platform brightness might change. Do you know which ones we want?

The documentation as I read it is saying that all of those are called when the appearance changes. I would implement only traitCollectionDidChange: (which best matches the conceptual use here) until/unless you have evidence that there are cases where that isn't called.

I removed the other messages. What about when the VC is first created?

I added a viewDidLoad implementation and called onUserSettingsChanged:nil there to ensure we have the correct setting at the beginning.

viewDidAppear: is already calling onUserSettingsChanged:

Is there a risk of a timing issue there given that Flutter is running asynchronously? To me, viewDidAppear sounds like it might show the wrong brightness momentarily before Flutter updates..

Given that Flutter is running asynchronously there's a risk either way; willAppear and didAppear are almost certainly in the same event loop anyway. There was a long chat thread about that originally, and I thought there was a specific plan around how to make the initial frame correct. But calling it when the engine is first created is a good start.

matthew-carroll · 2019-08-09T02:51:18Z

That’s because the official docs list all those methods as places where a VC should handle any changes to the trait collection.

…

On Thu, Aug 8, 2019 at 7:46 PM stuartmorgan ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In shell/platform/darwin/ios/framework/Source/FlutterViewController.mm <#10791 (comment)>: > @@ -690,6 +695,11 @@ - (CGFloat)statusBarPadding { return CGRectIsNull(intersection) ? 0.0 : intersection.size.height; } +- (void)viewWillLayoutSubviews { + [super viewWillLayoutSubviews]; + [self onUserSettingsChanged:nil]; Looking more broadly at the patch rather than just the part that was failing before: why are all these layout-based methods getting onUserSettingChanged: calls? traitCollectionDidChange: makes sense, but I don't understand why a relayout would change any settings (and especially why each relayout would need to update the settings 3 times, as this change appears to be doing.) — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#10791?email_source=notifications&email_token=ABXMHHHIJWI3CZO3W2V4CCLQDTK67A5CNFSM4IKPOT3KYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCBCHU7A#pullrequestreview-272923260>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABXMHHEVNE3U2NEHM6TDJ43QDTK67ANCNFSM4IKPOT3A> .

matthew-carroll · 2019-08-09T21:47:42Z

@chinmaygarde @stuartmorgan @dnfield @LongCatIsLooong can I merge in?

chinmaygarde

I would rather have a single policy for ARC in our codebase. It is extremely hard to review code when you don't know whether the missing dealloc is going to cause a leak or not because the flags the specific TU is compiled are not readily available to check.

But, I believe some of the Mac stuff already has ARC enabled TUs. So I can't fault this target for the its use of ARC either. We should figure out a migration to ARC soon though. This is getting really messy.

chinmaygarde · 2019-08-09T23:26:04Z

shell/platform/darwin/ios/framework/Source/FlutterViewController.mm

@@ -403,6 +403,10 @@ - (void)surfaceUpdated:(BOOL)appeared {

 #pragma mark - UIViewController lifecycle notifications

+- (void)viewDidLoad {


This needs to call [super viewDidLoad] I think.

This call is unlikely to do anything where it is, because the engine is created in viewWillAppear:, which is after this. So the channel will be nil, and the message will go nowhere.

This should be right after the launchEngine: call just below (and in the initWithEngine: for that path)

matthew-carroll · 2019-08-13T21:00:27Z

@stuartmorgan I moved the sending of user settings to the location you suggested.

FYI @darrenaustin @chinmaygarde @stuartmorgan I added the concept of platform contrast to this PR. We'll need it for Dark Mode, too, and it's essentially the same behavior that this PR added for platform brightness.

Please let me know if you think we're good to merge in with these changes.

stuartmorgan-g · 2019-08-13T22:29:11Z

shell/platform/darwin/ios/framework/Source/FlutterViewController.mm

+  if (@available(iOS 13, *)) {
+    UIAccessibilityContrast contrast = self.traitCollection.accessibilityContrast;
+
+    if (contrast == UIAccessibilityContrastHigh) {


This constant only exists in the iOS 13 SDK, unlike UIUserInterfaceStyleDark which is in 12. This isn't going to compile on the bots.

If you want to add this we need to go back to the early discussion of declaring the constant yourself, conditionally, behind an ifdef based on the compiling SDK, so that you can use the constant regardless of which SDK you're compiling against. Same with adding a declaration of accessibilityContrast, which is also 13+

Do you have an example of defining such a constant that I can reference and replicate?

I took a stab at only including the accessibility logic when the max API is 13 or above. Please let me know if that's sufficient.

As previously discussed, putting the actual implementation behind ifdefs doesn't do what you want, because that would mean it would only work if someone compiles their own engine locally with the Xcode beta; none of this code would actually be shipped in our library.

If this code is important to have now, then per the previous discussion what you need are declarations of the constant and the method that are behind reversed SDK ifdefs (i.e., the local declarations are used when compiling with a stable SDK, and not there to cause duplicate definition errors when using the beta SDK), allowing all of the logic here to compile regardless of SDK.

I don't have an example offhand, and would have to dig one up tomorrow. But the declarations that you need behind the ifdefs are exactly the ones that are in the beta SDK: the same name and value for the constant, and the identical signature for the method (declared via a class extension).

An example to replicate would be great. I wasn't clear on the distinction last time and I haven't been able to locate an example of it in practice.

Here are some examples; they are old and on macOS, but the principle is exactly the same.
method: https://hg.mozilla.org/camino/file/d974e1a3e6f55d09feb5afdf5ee50aa97f846518/src/browser/BrowserWindowController.mm#l108
constant: https://hg.mozilla.org/camino/rev/3249ec97333c#l3.12

stuartmorgan-g · 2019-08-13T22:35:52Z

shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest.m

+
+- (void)testItReportsPlatformBrightnessWhenViewWillAppear {
+  // Setup test.
+  id engine = OCMClassMock([FlutterEngine class]);


Have you verified that engine won't outlive this test? Per offline discussion, if a class mock outlives the test, Bad Things happen. I think the recent refactors to method channel handling means that it won't leak the engine like it used to, but I'd like to be sure.

How might I verify this?

I just went ahead and explicitly cleaned up every mock in each test.

…hanges in VC.

… available.

…cleaned up mocks in tests.

matthew-carroll · 2019-08-15T03:38:44Z

@stuartmorgan I pushed an update that hopefully contains the correct implementation of the conditional property inclusion that we discussed today. This appears to compile and run through tests without issue locally against 12.2 and 13. Please let me know if you think we're good to merge in with this round of changes.

stuartmorgan-g

LGTM with style nits

stuartmorgan-g · 2019-08-15T16:17:26Z

shell/platform/darwin/ios/framework/Source/FlutterViewController.mm

+  UIAccessibilityContrastHigh = 2
+} UIAccessibilityContrast;
+
+@interface UITraitCollection (AccessibilityContrastApi)


Nit: I'd suggest naming the extension something like MethodsFromNewerSDK, rather than AccessibilityContrastApi, to make it very obvious that this is a declaration of something from the SDK rather than a normal extension (with an implementation).

If you do keep the current name, s/Api/API/ per ObjC naming style.

shell/platform/darwin/ios/framework/Source/FlutterViewController.mm

stuartmorgan-g · 2019-08-15T16:25:15Z

shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest.m

+  [mockTraitCollection stopMocking];
+}
+
+- (UITraitCollection*)setupFakeTraitCollectionWithContrast:(UIAccessibilityContrast)contrast {


s/setupFakeTrait.../fakeTrait.../

This isn't operating on a trait collection, it's returning one, so should be named like a getter.

Also, this should get a brief declaration comment as well.

…ntrast (flutter/engine#10791)

git@github.com:flutter/engine.git/compare/f8e7453f1106...4b7a552 git log f8e7453..4b7a552 --no-merges --oneline 2019-08-16 stuartmorgan@google.com Roll Dart back to e35e8833 (flutter/engine#11043) 2019-08-16 bkonyi@google.com Roll src/third_party/dart 306f8e04bb..fecc4c8f2d (4 commits) 2019-08-16 skia-flutter-autoroll@skia.org Roll src/third_party/skia 963a606677e1..6a519b8dd895 (6 commits) (flutter/engine#11042) 2019-08-15 liyuqian@google.com Hide verbose dart snapshot during run_test.py (flutter/engine#11040) 2019-08-15 chinmaygarde@google.com Remove ability to override mac_sdk_path in flutter/tools/gn (flutter/engine#11013) 2019-08-15 bkonyi@google.com Roll src/third_party/dart cd16fba718..306f8e04bb (10 commits) 2019-08-15 stuartmorgan@google.com Roll buildroot to pick up recent macOS changes (flutter/engine#11037) 2019-08-15 jason-simmons@users.noreply.github.com Remove the ParagraphImpl class from the text API (flutter/engine#11012) 2019-08-15 jason-simmons@users.noreply.github.com Disable a deprecation warning for use of a TaskDescription constructor for older platforms (flutter/engine#11029) 2019-08-15 matthew-carroll@users.noreply.github.com Re-lands platform brightness support on iOS, plus platform contrast (flutter/engine#10791) 2019-08-15 iska.kaushik@gmail.com [fuchsia] Add required trace so files for fuchsia fars (flutter/engine#11036) 2019-08-15 skia-flutter-autoroll@skia.org Roll src/third_party/skia e5dc1ebc864a..963a606677e1 (14 commits) (flutter/engine#11032) 2019-08-15 stuartmorgan@google.com Add _glfw versions of the GLFW desktop libraries (flutter/engine#11024) 2019-08-15 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/linux-amd64 from _fvZN... to 5Nhwb... (flutter/engine#11028) 2019-08-15 bkonyi@google.com Roll src/third_party/dart 9552646dc4..cd16fba718 (5 commits) 2019-08-15 dnfield@google.com Fix first frame logic (flutter/engine#11027) 2019-08-15 dnfield@google.com remove OS version (flutter/engine#11033) 2019-08-15 skia-flutter-autoroll@skia.org Roll src/third_party/skia e30a485a68c9..e5dc1ebc864a (7 commits) (flutter/engine#11025) 2019-08-15 bkonyi@google.com Roll src/third_party/dart cae08c6813..9552646dc4 (3 commits) 2019-08-15 jason-simmons@users.noreply.github.com Remove the output directory prefix from the Android engine JAR filename (flutter/engine#11015) 2019-08-15 inthroxify@users.noreply.github.com Fix flutter/flutter #34791 (flutter/engine#9977) 2019-08-15 bkonyi@google.com Roll src/third_party/dart e35e8833ee..cae08c6813 (28 commits) 2019-08-15 skia-flutter-autoroll@skia.org Roll src/third_party/skia f3f50099533d..e30a485a68c9 (2 commits) (flutter/engine#11022) 2019-08-15 skia-flutter-autoroll@skia.org Roll src/third_party/skia 319fd3d7bcb4..f3f50099533d (4 commits) (flutter/engine#11021) The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff (stuartmorgan@google.com), and stop the roller if necessary.

matthew-carroll requested review from chinmaygarde, stuartmorgan-g and dnfield August 9, 2019 00:22

googlebot added the cla: yes label Aug 9, 2019

chinmaygarde suggested changes Aug 9, 2019

View reviewed changes

stuartmorgan-g reviewed Aug 9, 2019

View reviewed changes

matthew-carroll requested a review from chinmaygarde August 9, 2019 00:39

chinmaygarde reviewed Aug 9, 2019

View reviewed changes

stuartmorgan-g reviewed Aug 9, 2019

View reviewed changes

matthew-carroll requested a review from chinmaygarde August 9, 2019 21:04

chinmaygarde approved these changes Aug 9, 2019

View reviewed changes

stuartmorgan-g reviewed Aug 13, 2019

View reviewed changes

matthew-carroll added 11 commits August 14, 2019 16:56

Forwards iOS dark mode trait to the Flutter framework (flutter#34441).

05ad20c

Formatting update.

1bf2e6a

Removed unnecessary calls to onUserSettingsChanged for Dark Mode.

6126ae2

Formatting update.

27af2f8

Added tests.

74148fc

Format update.

e0b9656

Format update.

7bd9ac8

Add #ifdef to avoid CI falures due to iOS 13.

201f691

Changed #elif to #else

e1f0a20

Switch to local VC traitCollection instead of singleton.

55af42e

Changed test simulator back to OS 12.2

b709f95

matthew-carroll added 10 commits August 14, 2019 16:56

Removed all new user settings messages except when trait collection c…

ed60b1c

…hanges in VC.

Added user settings call to viewDidLoad in VC.

8fdb300

Enabled ARC for test.

ee5e8b8

Formatting.

5c83334

Formatting.

5ac2ada

Fixed test mocking mistake, added platform contrast to this PR.

435a134

Fixed formatting, returned early from contrast tests if iOS 13 is not…

13cfeaf

… available.

Formatting.

e53c791

Surrounded accessibility contrast with conditional preprocessors and …

c0de8e5

…cleaned up mocks in tests.

Adjusted to be compatible with API 12.2

c76d20a

matthew-carroll force-pushed the 34441_send_ios_brightness_to_flutter branch from 160e3b8 to c76d20a Compare August 15, 2019 03:08

Formatting

557fb06

stuartmorgan-g approved these changes Aug 15, 2019

View reviewed changes

matthew-carroll added 4 commits August 15, 2019 13:18

PR Updates.

7bfdd78

Formatting

a4053eb

Formatting

1921433

Reverted run_tests.h device type.

0e2a1b5

matthew-carroll merged commit 5bb144c into flutter:master Aug 15, 2019

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 16, 2019

5bb144c Re-lands platform brightness support on iOS, plus platform co…

d8fef04

…ntrast (flutter/engine#10791)

engine-flutter-autoroll mentioned this pull request Aug 16, 2019

Roll engine f8e7453f1106..4b7a552b9eae (24 commits) flutter/flutter#38663

Merged

This was referenced Sep 11, 2019

Planned Cupertino Library Features Jul-Sept 2019 flutter/flutter#35026

Closed

[iOS 13] add support for detecting device dark mode enabled flutter/flutter#33873

Closed

LongCatIsLooong mentioned this pull request Jan 18, 2020

Add support for Increase Contrast on iOS #15343

Merged

		@@ -403,6 +403,10 @@ - (void)surfaceUpdated:(BOOL)appeared {

		#pragma mark - UIViewController lifecycle notifications

		- (void)viewDidLoad {

Re-lands platform brightness support on iOS #10791

Re-lands platform brightness support on iOS #10791

Conversation

matthew-carroll commented Aug 9, 2019

Uh oh!

chinmaygarde left a comment • 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!

matthew-carroll commented Aug 9, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matthew-carroll commented Aug 9, 2019

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!

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!

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!

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!

matthew-carroll commented Aug 9, 2019 via email

Uh oh!

matthew-carroll commented Aug 9, 2019

Uh oh!

chinmaygarde 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!

matthew-carroll commented Aug 13, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chinmaygarde left a comment •

edited

Loading