-
Notifications
You must be signed in to change notification settings - Fork 509
test: 🧪 Added tests for Overlay #549
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
base: master
Are you sure you want to change the base?
Conversation
860c94b
to
aa84c53
Compare
aa84c53
to
6b366eb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Additional Note -
Please cover following cases:
- Showcaseview properties applied correctly to the overlay
- Only the properties of first showcase is used for the overlay in case of simultaneously showing multiple showcases
- The overlay has correct rect cutouts for the target widget
lib/src/utils/extensions.dart
Outdated
Color reduceOpacity(double opacity) => withAlpha((opacity * 255).round()); | ||
|
||
/// Safe replacement for deprecated red property | ||
int get safeRed => (0x00ff0000 & _toARGB32()) >> 16; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please mention the reason behind creating this method. In which flutter version did the property got deprecated and in what flutter version it would be safe to remove this and use inbuilt way.
|
||
double? findOverlayBlurSigma(WidgetTester tester) { | ||
final filters = | ||
tester.widgetList(find.byType(ImageFiltered)).cast<ImageFiltered?>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use the generic type of widgetList
instead of casting.
|
||
final match = RegExp(r'ImageFilter\.blur\(\s*([0-9.]+)\s*,\s*([0-9.]+)') | ||
.firstMatch(str); | ||
if (match != null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use if case.
setUp( | ||
() { | ||
ShowcaseView.register(); | ||
}, | ||
); | ||
tearDown( | ||
() { | ||
ShowcaseView.get().unregister(); | ||
}, | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please leverage tear off.
); | ||
|
||
testWidgets( | ||
'Overlay renders with default properties', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test and description doesn't match. No overlay properties are checked for default value.
expect(find.byType(Overlay), findsWidgets); | ||
|
||
// Explicitly ask to rebuild the overlay | ||
ShowcaseView.get().updateOverlay(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't be required.
}, | ||
); | ||
|
||
testWidgets('Overlay with custom target shape borders', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test and the description doesn't match. The border applied is not checked.
expect(find.byType(Overlay), findsWidgets); | ||
}); | ||
|
||
testWidgets('Overlay with zero opacity', (WidgetTester tester) async { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can combine this with different opacities tests.
expect(color, Colors.black.reduceOpacity(0.0)); | ||
}); | ||
|
||
testWidgets('Overlay with full opacity', (WidgetTester tester) async { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be combined as well as mentioned for the above test.
}); | ||
}); | ||
|
||
group('Overlay Edge Cases', () { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of the tests in this group can be combined with the test in the above group.
Description
Checklist
fix:
,feat:
,docs:
etc).docs
and added dartdoc comments with///
.examples
ordocs
.Breaking Change?
Related Issues