Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[google_sign_in] Use a transparent image as a placeholder for the GoogleUserCircleAvatar #4391

Merged
merged 17 commits into from
Oct 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/google_sign_in/google_sign_in/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 5.2.1

Change the placeholder of the GoogleUserCircleAvatar to a transparent image.

## 5.2.0

* Add `GoogleSignInAccount.serverAuthCode`. Mark `GoogleSignInAuthentication.serverAuthCode` as deprecated.
Expand Down
14 changes: 13 additions & 1 deletion packages/google_sign_in/google_sign_in/lib/widgets.dart
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,22 @@ class GoogleUserCircleAvatar extends StatelessWidget {
FadeInImage.memoryNetwork(
// This creates a transparent placeholder image, so that
// [placeholder] shows through.
placeholder: Uint8List((size.round() * size.round())),
placeholder: _transparentImage,
image: sizedPhotoUrl,
)
]),
));
}
}

/// This is an transparent 1x1 gif image.
///
/// Those bytes come from `resources/transparentImage.gif`.
final Uint8List _transparentImage = Uint8List.fromList(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should check in a script that generates this code and add a comment here pointing to it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry, I'm a bit confused and I don't understand what you want me to do?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should check in a script that generates this code

Why? Under what circumstance would the bytes of a static image need to be re-generated?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Presumably a script was already written to generate it. Who knows, maybe there is a bug where the pixel is #000000FF but we need #FFFFFFFF, depending on alpha blending that can make a difference. Plus it means people don't need to execute the code to know it is correct. I don't know the gif header format.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternately, we could compare it byte by byte to the source, which would be significantly less work than reviewing (and then maintaining) a script.

@ValentinVignal Can you add a comment with the URL of the source gif you used?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. So let's land the gif (maybe in a new package-level directory with a README explaining why it's there), and reference it in a comment in the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello, I'm sorry again, I'm still not clear on what I should do exactly. Let me try to reformulate and you tell me if I'm wrong?


I should:

  1. Create a new package in plugin/packages/<new package>
  2. Add the gif image as an asset and this variable transparentImage (not private)
  3. Import this package in google_sign_in to reuse the variable transparentImage
  4. Write a test in the new repo to read the bytes of the gif image and compare them to the ones in the variable

Did I miss something?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, you just need to add the source GIF to this PR. My suggestion is that it be in a subdirectory (e.g., resources/) with a README, rather than being at the top level of the plugin.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stuartmorgan @gaaclarke I couldn't remember where I took the image in the first place.

In 4d17090, I used this one:

from this wikimedia page.

I added the image in the repo as resources/transparentImage.gif and updated the bytes of _transparentImage to match the gif file.

I got the bytes by doing:

final bytes = await File('resources/transparentImage.gif').readAsBytes();

Is it what you were requesting?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, that sounds good to me, thanks!

[
0x47, 0x49, 0x46, 0x38, 0x39, 0x61, 0x01, 0x00, 0x01, 0x00, 0x80, 0x00, //
0x00, 0x00, 0x00, 0x00, 0xFF, 0xFF, 0xFF, 0x21, 0xf9, 0x04, 0x01, 0x00, //
0x00, 0x00, 0x00, 0x2C, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, //
0x00, 0x02, 0x01, 0x44, 0x00, 0x3B
],
);
2 changes: 1 addition & 1 deletion packages/google_sign_in/google_sign_in/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ description: Flutter plugin for Google Sign-In, a secure authentication system
for signing in with a Google account on Android and iOS.
repository: https://github.com/flutter/plugins/tree/master/packages/google_sign_in/google_sign_in
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+google_sign_in%22
version: 5.2.0
version: 5.2.1

environment:
sdk: ">=2.14.0 <3.0.0"
Expand Down
7 changes: 7 additions & 0 deletions packages/google_sign_in/google_sign_in/resources/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
`transparentImage.gif` is a 1x1 transparent gif which comes from [this wikimedia page](https://commons.wikimedia.org/wiki/File:Transparent.gif):

![](transparentImage.gif)

This is the image used a placeholder for the `GoogleCircleAvatar` widget.

The variable `_transparentImage` in `lib/widgets.dart` is the list of bytes of `transparentImage.gif`.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
117 changes: 117 additions & 0 deletions packages/google_sign_in/google_sign_in/test/widgets_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'dart:async';
import 'dart:io';
import 'dart:typed_data';

import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:google_sign_in/google_sign_in.dart';

/// A instantiable class that extends [GoogleIdentity]
class _TestGoogleIdentity extends GoogleIdentity {
_TestGoogleIdentity({
required this.id,
required this.email,
this.photoUrl,
});

final String id;
final String email;

final String? photoUrl;

@override
String? get displayName => null;

@override
String? get serverAuthCode => null;
}

/// A mocked [HttpClient] which always returns a [_MockHttpRequest].
class _MockHttpClient extends Fake implements HttpClient {
@override
bool autoUncompress = true;

@override
Future<HttpClientRequest> getUrl(Uri url) {
return Future<HttpClientRequest>.value(_MockHttpRequest());
}
}

/// A mocked [HttpClientRequest] which always returns a [_MockHttpClientResponse].
class _MockHttpRequest extends Fake implements HttpClientRequest {
@override
Future<HttpClientResponse> close() {
return Future<HttpClientResponse>.value(_MockHttpResponse());
}
}

/// Arbitrary valid image returned by the [_MockHttpResponse].
///
/// This is an transparent 1x1 gif image.
/// It doesn't have to match the placeholder used in [GoogleUserCircleAvatar].
///
/// Those bytes come from `resources/transparentImage.gif`.
final Uint8List _transparentImage = Uint8List.fromList(
[
0x47, 0x49, 0x46, 0x38, 0x39, 0x61, 0x01, 0x00, 0x01, 0x00, 0x80, 0x00, //
0x00, 0x00, 0x00, 0x00, 0xFF, 0xFF, 0xFF, 0x21, 0xf9, 0x04, 0x01, 0x00, //
0x00, 0x00, 0x00, 0x2C, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, //
0x00, 0x02, 0x01, 0x44, 0x00, 0x3B
],
);

/// A mocked [HttpClientResponse] which is empty and has a [statusCode] of 200
/// and returns valid image.
class _MockHttpResponse extends Fake implements HttpClientResponse {
final Stream<Uint8List> _delegate =
Stream<Uint8List>.value(_transparentImage);

@override
int get contentLength => -1;

@override
HttpClientResponseCompressionState get compressionState {
return HttpClientResponseCompressionState.decompressed;
}

@override
StreamSubscription<Uint8List> listen(void Function(Uint8List event)? onData,
{Function? onError, void Function()? onDone, bool? cancelOnError}) {
return _delegate.listen(onData,
onError: onError, onDone: onDone, cancelOnError: cancelOnError);
}

@override
int get statusCode => 200;
}

void main() {
testWidgets('It should build the GoogleUserCircleAvatar successfully',
(WidgetTester tester) async {
final GoogleIdentity identity = _TestGoogleIdentity(
email: '[email protected]',
id: 'userId',
photoUrl: 'photoUrl',
);
tester.binding.window.physicalSizeTestValue = Size(100, 100);

await HttpOverrides.runZoned(
() async {
await tester.pumpWidget(MaterialApp(
home: SizedBox(
height: 100,
width: 100,
child: GoogleUserCircleAvatar(
identity: identity,
),
),
));
},
createHttpClient: (SecurityContext? c) => _MockHttpClient(),
);
});
}