Skip to content

simple code QA for a better scoring #379

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

Merged
merged 8 commits into from
Mar 15, 2024
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
2 changes: 1 addition & 1 deletion .all-contributorsrc
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@
"login": "gnunicorn",
"name": "Benjamin Kampmann",
"avatar_url": "https://avatars.githubusercontent.com/u/40496?v=4",
"profile": "http://www.gnunicorn.org",
"profile": "https://gnunicorn.org",
"contributions": [
"code"
]
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ jobs:
working-directory: packages/${{ matrix.package_name_and_runs_on.package_name }}

- name: Check format
run: dart format --set-exit-if-changed --line-length 120 .
run: dart format --set-exit-if-changed .
working-directory: packages/${{ matrix.package_name_and_runs_on.package_name }}

# TODO fix it (seems GitHub Actions upgraded, making Fastlane confused)
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ For detailed PRs (excluding myself), please see [this link](https://github.com/f
<td align="center" valign="top" width="14.28%"><a href="https://github.com/gilice"><img src="https://avatars.githubusercontent.com/u/104317939?v=4?s=100" width="100px;" alt="gilice"/><br /><sub><b>gilice</b></sub></a><br /><a href="https://github.com/fzyzcjy/flutter_convenient_test/commits?author=gilice" title="Code">💻</a></td>
<td align="center" valign="top" width="14.28%"><a href="https://github.com/madmini"><img src="https://avatars.githubusercontent.com/u/26089559?v=4?s=100" width="100px;" alt="Max Jakobitsch"/><br /><sub><b>Max Jakobitsch</b></sub></a><br /><a href="https://github.com/fzyzcjy/flutter_convenient_test/commits?author=madmini" title="Code">💻</a></td>
<td align="center" valign="top" width="14.28%"><a href="https://keybase.io/jmatth"><img src="https://avatars.githubusercontent.com/u/1316184?v=4?s=100" width="100px;" alt="Josh Matthews"/><br /><sub><b>Josh Matthews</b></sub></a><br /><a href="https://github.com/fzyzcjy/flutter_convenient_test/commits?author=jmatth" title="Code">💻</a></td>
<td align="center" valign="top" width="14.28%"><a href="http://www.gnunicorn.org"><img src="https://avatars.githubusercontent.com/u/40496?v=4?s=100" width="100px;" alt="Benjamin Kampmann"/><br /><sub><b>Benjamin Kampmann</b></sub></a><br /><a href="https://github.com/fzyzcjy/flutter_convenient_test/commits?author=gnunicorn" title="Code">💻</a></td>
<td align="center" valign="top" width="14.28%"><a href="https://gnunicorn.org"><img src="https://avatars.githubusercontent.com/u/40496?v=4?s=100" width="100px;" alt="Benjamin Kampmann"/><br /><sub><b>Benjamin Kampmann</b></sub></a><br /><a href="https://github.com/fzyzcjy/flutter_convenient_test/commits?author=gnunicorn" title="Code">💻</a></td>
<td align="center" valign="top" width="14.28%"><a href="https://github.com/meltzow"><img src="https://avatars.githubusercontent.com/u/1238001?v=4?s=100" width="100px;" alt="Mario Meltzow"/><br /><sub><b>Mario Meltzow</b></sub></a><br /><a href="https://github.com/fzyzcjy/flutter_convenient_test/commits?author=meltzow" title="Code">💻</a></td>
</tr>
</tbody>
Expand Down
38 changes: 29 additions & 9 deletions packages/convenient_test/example/integration_test/main_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -68,14 +68,17 @@ void main() {
tTestWidgets('golden test', (t) async {
await find.text('HomePage').should(findsOneWidget);

await find.byType(MaterialApp).should(matchesGoldenFile('goldens/sample_golden.png'));
await find
.byType(MaterialApp)
.should(matchesGoldenFile('goldens/sample_golden.png'));
});

tTestWidgets('deliberately failed golden test', (t) async {
await t.visit('/random');
await find.text('RandomPage').should(findsOneWidget);

await find.get(RandomPageMark.randomText).should(matchesGoldenFile('goldens/deliberately_failed_golden.png'));
await find.get(RandomPageMark.randomText).should(
matchesGoldenFile('goldens/deliberately_failed_golden.png'));

// let's assert something else
await find.textContaining('Random Height').should(findsOneWidget);
Expand Down Expand Up @@ -146,11 +149,15 @@ void main() {
await t.visit('/zoom');
await t.pumpAndSettle();

await find.get(ZoomPageMark.palette).should(matchesGoldenFile('goldens/zoom_page_drag_before.png'));
await find
.get(ZoomPageMark.palette)
.should(matchesGoldenFile('goldens/zoom_page_drag_before.png'));

await find.get(ZoomPageMark.palette).drag(const Offset(0, -50));

await find.get(ZoomPageMark.palette).should(matchesGoldenFile('goldens/zoom_page_drag_after.png'));
await find
.get(ZoomPageMark.palette)
.should(matchesGoldenFile('goldens/zoom_page_drag_after.png'));

// alternative approach
// await t.tester.drag(find.get(ZoomPageMark.palette), const Offset(0, -50));
Expand All @@ -163,17 +170,29 @@ void main() {
tTestWidgets('double finger zooming', (t) async {
await t.visit('/zoom');

await find.get(ZoomPageMark.palette).should(matchesGoldenFile('goldens/zoom_page_zoom_before.png'));
await find
.get(ZoomPageMark.palette)
.should(matchesGoldenFile('goldens/zoom_page_zoom_before.png'));

await find.get(ZoomPageMark.palette).multiDrag(
firstDownOffset: const Offset(0, -30),
secondDownOffset: const Offset(0, 30),
firstFingerOffsets: const [Offset(0, -20), Offset(0, -20), Offset(0, -10)],
secondFingerOffsets: const [Offset(0, 20), Offset(0, 20), Offset(0, 10)],
firstFingerOffsets: const [
Offset(0, -20),
Offset(0, -20),
Offset(0, -10)
],
secondFingerOffsets: const [
Offset(0, 20),
Offset(0, 20),
Offset(0, 10)
],
logMove: true,
);

await find.get(ZoomPageMark.palette).should(matchesGoldenFile('goldens/zoom_page_zoom_after.png'));
await find
.get(ZoomPageMark.palette)
.should(matchesGoldenFile('goldens/zoom_page_zoom_after.png'));
});
});
});
Expand All @@ -199,7 +218,8 @@ class MyConvenientTestSlot extends ConvenientTestSlot {
Future<void> appMain(AppMainExecuteMode mode) async => app.main();

@override
BuildContext? getNavContext(ConvenientTest t) => MyApp.navigatorKey.currentContext;
BuildContext? getNavContext(ConvenientTest t) =>
MyApp.navigatorKey.currentContext;
}

extension on ConvenientTest {
Expand Down
14 changes: 10 additions & 4 deletions packages/convenient_test/example/lib/home_page.dart
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ class _HomePageState extends State<HomePage> {
floatingActionButton: Mark(
name: HomePageMark.fab,
child: FloatingActionButton(
onPressed: () => Navigator.pushNamed(context, '/second', arguments: chosenFruits),
onPressed: () =>
Navigator.pushNamed(context, '/second', arguments: chosenFruits),
child: const Icon(Icons.done),
),
),
Expand Down Expand Up @@ -51,9 +52,12 @@ class _HomePageState extends State<HomePage> {
return Column(
children: [
Padding(
padding: const EdgeInsets.only(top: 16, bottom: 4, left: 16, right: 16),
padding:
const EdgeInsets.only(top: 16, bottom: 4, left: 16, right: 16),
child: Text(
chosenFruits.isEmpty ? 'You chose nothing' : 'You chose: ${chosenFruits.join(', ')}',
chosenFruits.isEmpty
? 'You chose nothing'
: 'You chose: ${chosenFruits.join(', ')}',
style: const TextStyle(fontSize: 16),
),
),
Expand All @@ -65,7 +69,9 @@ class _HomePageState extends State<HomePage> {
return ListTile(
onTap: () => setState(() => chosenFruits.toggle(fruit)),
title: Text(fruit),
trailing: chosenFruits.contains(fruit) ? const Icon(Icons.star, color: Colors.blue) : null,
trailing: chosenFruits.contains(fruit)
? const Icon(Icons.star, color: Colors.blue)
: null,
);
},
),
Expand Down
8 changes: 6 additions & 2 deletions packages/convenient_test/example/lib/second_page.dart
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,12 @@ class SecondPage extends StatelessWidget {
Row(
mainAxisSize: MainAxisSize.min,
children: [
TextButton(onPressed: () => Navigator.pushNamed(context, '/timer'), child: const Text('GoTimerPage')),
TextButton(onPressed: () => Navigator.pushNamed(context, '/zoom'), child: const Text('GoZoomPage')),
TextButton(
onPressed: () => Navigator.pushNamed(context, '/timer'),
child: const Text('GoTimerPage')),
TextButton(
onPressed: () => Navigator.pushNamed(context, '/zoom'),
child: const Text('GoZoomPage')),
],
),
],
Expand Down
3 changes: 2 additions & 1 deletion packages/convenient_test/example/lib/zoom_page.dart
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ class _ZoomPageState extends State<ZoomPage> {
child: Container(
width: 50,
height: 50,
color: _getColor(Point(x, y), invert: _chosenPoint == Point(x, y)),
color: _getColor(Point(x, y),
invert: _chosenPoint == Point(x, y)),
child: Center(
child: Text('$x#$y'),
),
Expand Down
12 changes: 9 additions & 3 deletions packages/convenient_test/lib/src/gesture_visualizer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ import 'package:flutter/material.dart';
import 'package:flutter/rendering.dart';

/// These are the parameters for the visualization of the recorded taps.
final _kTapRadius = 15.0, _kTapColor = Colors.grey[300]!, _kShadowColor = Colors.black, _kShadowElevation = 3.0;
final _kTapRadius = 15.0,
_kTapColor = Colors.grey[300]!,
_kShadowColor = Colors.black,
_kShadowElevation = 3.0;
const _kRemainAfterPointerUp = Duration(milliseconds: 100);

/// NOTE: refer to this answer for why use hitTest/handleEvent/etc https://stackoverflow.com/a/65067655
Expand Down Expand Up @@ -57,7 +60,8 @@ class _RenderGestureVisualizer extends RenderProxyBox {
markNeedsPaint();
});
} else {
_recordedPointerInfoMap[event.pointer] = _RecordedPointerInfo(event.localPosition);
_recordedPointerInfoMap[event.pointer] =
_RecordedPointerInfo(event.localPosition);
markNeedsPaint();
}
}
Expand All @@ -68,7 +72,9 @@ class _RenderGestureVisualizer extends RenderProxyBox {

final canvas = context.canvas;
for (final info in _recordedPointerInfoMap.values) {
final path = Path()..addOval(Rect.fromCircle(center: info.localPosition, radius: _kTapRadius));
final path = Path()
..addOval(
Rect.fromCircle(center: info.localPosition, radius: _kTapRadius));

canvas.drawShadow(path, _kShadowColor, _kShadowElevation, true);
canvas.drawPath(path, Paint()..color = _kTapColor);
Expand Down
5 changes: 4 additions & 1 deletion packages/convenient_test/lib/src/misc.dart
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,10 @@ class Mark extends StatelessWidget {
}

String _onlyUpperOrFirstLetter(String s) {
return s.split('').whereIndexed((i, ch) => i == 0 || ch.toUpperCase() == ch).join('');
return s
.split('')
.whereIndexed((i, ch) => i == 0 || ch.toUpperCase() == ch)
.join('');
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,17 @@ import 'package:mobx/mobx.dart';

mixin AttachableStateMixin<T> {
T get state {
assert(isStateSingle, 'Want to get this.state but wrong number of _states. $attachableStateMixinInfo');
assert(isStateSingle,
'Want to get this.state but wrong number of _states. $attachableStateMixinInfo');
return _states.single;
}

int get numStates => _states.length;

bool get isStateSingle => numStates == 1;

String get attachableStateMixinInfo => 'AttachableStateMixin(runtimeType=$runtimeType, hash=$hashCode, '
String get attachableStateMixinInfo =>
'AttachableStateMixin(runtimeType=$runtimeType, hash=$hashCode, '
'_states=$_states (their hashCodes: ${_states.map((s) => s.hashCode).toList()}))';

List<T> get states => _states;
Expand All @@ -32,7 +34,8 @@ mixin AttachableStateMixin<T> {
}
}

class AttachableStateAttacherWidget<T extends AttachableStateMixin<S>, S> extends StatefulWidget {
class AttachableStateAttacherWidget<T extends AttachableStateMixin<S>, S>
extends StatefulWidget {
final T target;
final S state;
final Widget child;
Expand All @@ -45,7 +48,8 @@ class AttachableStateAttacherWidget<T extends AttachableStateMixin<S>, S> extend
});

@override
_AttachableStateAttacherWidgetState<T, S> createState() => _AttachableStateAttacherWidgetState<T, S>();
_AttachableStateAttacherWidgetState<T, S> createState() =>
_AttachableStateAttacherWidgetState<T, S>();
}

class _AttachableStateAttacherWidgetState<T extends AttachableStateMixin<S>, S>
Expand All @@ -57,9 +61,11 @@ class _AttachableStateAttacherWidgetState<T extends AttachableStateMixin<S>, S>
}

@override
void didUpdateWidget(covariant AttachableStateAttacherWidget<T, S> oldWidget) {
void didUpdateWidget(
covariant AttachableStateAttacherWidget<T, S> oldWidget) {
super.didUpdateWidget(oldWidget);
if (!identical(oldWidget.target, widget.target) || !identical(oldWidget.state, widget.state)) {
if (!identical(oldWidget.target, widget.target) ||
!identical(oldWidget.state, widget.state)) {
oldWidget.target.detach(oldWidget.state);
widget.target.attach(widget.state);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ import 'dart:ui' as ui;
import 'package:flutter/material.dart';
import 'package:flutter/scheduler.dart';

typedef ImageDecoderBuilder<Tag> = Widget Function(BuildContext context, TaggedImageInfo<Tag>? info);
typedef ImageDecoderBuilder<Tag> = Widget Function(
BuildContext context, TaggedImageInfo<Tag>? info);

class ImageDecoderWidget<Tag> extends StatefulWidget {
const ImageDecoderWidget({
Expand Down Expand Up @@ -42,7 +43,8 @@ class ImageDecoderWidget<Tag> extends StatefulWidget {
final void Function(ui.Image? image)? onUiImage;

@override
State<ImageDecoderWidget<Tag>> createState() => _ImageDecoderWidgetState<Tag>();
State<ImageDecoderWidget<Tag>> createState() =>
_ImageDecoderWidgetState<Tag>();
}

class _ImageDecoderWidgetState<Tag> extends State<ImageDecoderWidget<Tag>> {
Expand Down Expand Up @@ -70,19 +72,22 @@ class _ImageDecoderWidgetState<Tag> extends State<ImageDecoderWidget<Tag>> {
final interestImageProvider = widget.imageProvider;
final oldImageStreamWithListener = _imageStreamWithListener;
_imageStreamWithListener = _ImageStreamWithListener(
interestImageProvider.inner.resolve(createLocalImageConfiguration(context)),
interestImageProvider.inner
.resolve(createLocalImageConfiguration(context)),
ImageStreamListener(
(imageInfo, synchronousCall) =>
_updateImage(imageInfo, synchronousCall, imageProviderTag: interestImageProvider.tag),
(imageInfo, synchronousCall) => _updateImage(imageInfo, synchronousCall,
imageProviderTag: interestImageProvider.tag),
// NOTE XXX add
onError: widget.onError,
),
);

if (oldImageStreamWithListener != null) {
oldImageStreamWithListener.inner.removeListener(oldImageStreamWithListener.listener);
oldImageStreamWithListener.inner
.removeListener(oldImageStreamWithListener.listener);
}
_imageStreamWithListener!.inner.addListener(_imageStreamWithListener!.listener);
_imageStreamWithListener!.inner
.addListener(_imageStreamWithListener!.listener);

// NOTE XXX edit
// if (_imageStream!.key != oldImageStream?.key) {
Expand All @@ -95,15 +100,18 @@ class _ImageDecoderWidgetState<Tag> extends State<ImageDecoderWidget<Tag>> {
// }
}

void _updateImage(ImageInfo imageInfo, bool synchronousCall, {required Tag imageProviderTag}) {
void _updateImage(ImageInfo imageInfo, bool synchronousCall,
{required Tag imageProviderTag}) {
// print('hi _updateImage imageInfo=$imageInfo');
setState(() {
// Trigger a build whenever the image changes.
// NOTE MODIFIED 推迟dispose #5256
// _imageInfo?.inner.dispose();
final oldImageInfo = _imageInfo;
SchedulerBinding.instance.addPostFrameCallback((_) => oldImageInfo?.inner.dispose());
_imageInfo = TaggedImageInfo(imageInfo, imageProviderTag: imageProviderTag);
SchedulerBinding.instance
.addPostFrameCallback((_) => oldImageInfo?.inner.dispose());
_imageInfo =
TaggedImageInfo(imageInfo, imageProviderTag: imageProviderTag);
widget.onUiImage?.call(_imageInfo?.inner.image); // NOTE XXX add
});
}
Expand All @@ -112,7 +120,8 @@ class _ImageDecoderWidgetState<Tag> extends State<ImageDecoderWidget<Tag>> {
void dispose() {
// _imageStream?.removeListener(ImageStreamListener(_updateImage)); // NOTE XXX
if (_imageStreamWithListener != null) {
_imageStreamWithListener!.inner.removeListener(_imageStreamWithListener!.listener);
_imageStreamWithListener!.inner
.removeListener(_imageStreamWithListener!.listener);
}
_imageInfo?.inner.dispose();
_imageInfo = null;
Expand All @@ -134,7 +143,8 @@ class TaggedImageInfo<Tag> {
const TaggedImageInfo(this.inner, {required this.imageProviderTag});

@override
String toString() => 'TaggedImageInfo(inner: $inner, imageProviderTag: $imageProviderTag)';
String toString() =>
'TaggedImageInfo(inner: $inner, imageProviderTag: $imageProviderTag)';
}

@immutable
Expand All @@ -144,10 +154,12 @@ class TaggedImageProvider<Tag> {

const TaggedImageProvider(this.inner, {required this.tag});

static TaggedImageProvider<Tag>? maybe<Tag>(ImageProvider? inner, {required Tag tag}) =>
static TaggedImageProvider<Tag>? maybe<Tag>(ImageProvider? inner,
{required Tag tag}) =>
inner == null ? null : TaggedImageProvider(inner, tag: tag);

static TaggedImageProvider<Object?> noTag(ImageProvider inner) => TaggedImageProvider(inner, tag: null);
static TaggedImageProvider<Object?> noTag(ImageProvider inner) =>
TaggedImageProvider(inner, tag: null);
}

@immutable
Expand Down
Loading