-
Notifications
You must be signed in to change notification settings - Fork 271
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
Feat: ✨ Floating card animation #144
Conversation
aditya-css
commented
Sep 4, 2023
- Created flutter plugin structure.
- Added method and event channel for passing gyroscope sensor data from native side (ios and android).
- Added cursor_listener.dart for listening to cursor movement on desktop and website.
- Added floating_event.dart to represent the movement data.
- Added floating_controller.dart to house x and y axis points and have transformation logic to move the widget as per movement data.
- Updated readme.md to reflect latest addition of floating animation feature.
908ad8a
to
d9e58d1
Compare
README.md
Outdated
@@ -9,7 +9,9 @@ A Flutter package allows you to easily implement the Credit card's UI easily wit | |||
|
|||
## Preview | |||
|
|||
![The example app running in Android](https://github.com/simformsolutions/flutter_credit_card/blob/master/readme_assets/preview.gif) | |||
<img src="readme_assets/preview.gif" alt="The example app showing credit card widget" width="240" height="451"/> | |||
<img src="readme_assets/credit_card_float_preview.gif" alt="The example app showing card floating animation in mobile" width="207" height="448"/> |
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.
as discussed update gif
README.md
Outdated
@@ -109,6 +111,20 @@ import 'package:flutter_credit_card/flutter_credit_card.dart'; | |||
), | |||
``` | |||
|
|||
*Floating Card* | |||
|
|||
Make sure to call `CreditCardWidget.instance.initialize()` before using `CreditCardWidget` with |
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.
Let's try to handle this intialization internally. CreditCardWidget.instance.initialize()
android/build.gradle
Outdated
@@ -0,0 +1,64 @@ | |||
group 'com.simform.flutter_credit_card' | |||
version '3.0.7' |
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.
is this the package version? if yes can we make this dynamic?
import io.flutter.plugin.common.EventChannel | ||
import io.flutter.plugin.common.EventChannel.EventSink | ||
|
||
private const val FPS_60 = 16666 |
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 remove this and use static value directly
|
||
override fun onListen(arguments: Any?, events: EventSink) { | ||
sensorEventListener = createSensorEventListener(events) | ||
sensorManager.registerListener(sensorEventListener, sensor, FPS_60) |
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.
use value directly here.
@@ -0,0 +1,6 @@ | |||
package com.simform.flutter_credit_card_example |
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.
is this file necessary to add can we delete this file if not required?
d9e58d1
to
e64a327
Compare
e64a327
to
2146540
Compare
example/lib/main.dart
Outdated
import 'package:flutter_credit_card/credit_card_brand.dart'; | ||
import 'package:flutter_credit_card/flutter_credit_card.dart'; | ||
|
||
void main() => runApp(MySample()); | ||
Future<void> main() async { | ||
WidgetsFlutterBinding.ensureInitialized(); |
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.
any reason to add this code ?
example/lib/main.dart
Outdated
labelText: 'Expired Date', | ||
hintText: 'XX/XX', | ||
CreditCardWidget( | ||
shouldFloat: useFloatingAnimation, |
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.
instead of should use enable word. shouldFloat > enableFloatingCard
example/lib/main.dart
Outdated
hintText: 'XX/XX', | ||
CreditCardWidget( | ||
shouldFloat: useFloatingAnimation, | ||
shouldAddGlare: true, |
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 parameters are dependant on floating card so instead o separate parameter we can create a class like FloatingCardConfig
example/web/favicon.png
Outdated
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.
I suggest to use simform favicon here.
lib/credit_card_background.dart
Outdated
final double screenWidth = constraints.maxWidth.isInfinite | ||
? MediaQuery.of(context).size.width | ||
: constraints.maxWidth; | ||
final double screenHeight = MediaQuery.of(context).size.height; |
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.
take variable for MediaQuery.of(context).size
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.
Done in next PR where inheritedModel is utilised (i.e. MediaQuery.sizeOf()) as Dart 3 support is enabled in it.
lib/credit_card_background.dart
Outdated
.withOpacity(AppConstants.minShadowOpacity), | ||
offset: Offset( | ||
floatingController!.y * 100, | ||
-floatingController!.x * 100 + 8, |
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.
any specific reason to add 8 here? @kavantrivedi @aditya-chavda
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.
Just to have the shadow a below the card widget by 8 pixel. Want me to add comment there expressing the same?
lib/credit_card_widget.dart
Outdated
_gradientSetup(); | ||
_updateRotations(false); | ||
} | ||
|
||
@override | ||
void didChangeDependencies() { | ||
orientation = MediaQuery.of(context).orientation; |
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.
can we write this code like this ?
orientation ??= MediaQuery.of(context).orientation;
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.
I believe it would be better to always use the updated values of the MediaQuery
object.
? AppConstants.creditCardPadding | ||
: widget.padding; | ||
|
||
return CursorListener( |
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.
I belive this is only used as listner so can we avoid this widget from build and use some other workaround?
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.
That is correct, but it listens over a particular region, which would be the credit card widget, and thus it is in the build method.
edb21a4
to
2c12a36
Compare
|
||
override fun onListen(arguments: Any?, events: EventSink) { | ||
sensorEventListener = createSensorEventListener(events) | ||
sensorManager.registerListener(sensorEventListener, sensor, 16666) |
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 constant instead of directly as a value. Also document that what this constant does.
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.
Conflicts with @vatsaltanna 's request to revert using const variable for that value.
|
||
init(motionManager: CMMotionManager) { | ||
self.motionManager = motionManager | ||
self.motionManager!.gyroUpdateInterval = 0.016666 |
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 constant instead of directly as a value. Also document that what this constant does.
Also have you taken into account higher refresh rate devices?
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.
@vatsaltanna please advice on accounting device's refresh rate instead of fixed 60 fps.
color: shadowConfig!.color, | ||
offset: shadowConfig!.offset + | ||
Offset( | ||
floatingController!.y * 100, |
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.
Use contant.
lib/credit_card_widget.dart
Outdated
|
||
bool isAnimating = false; | ||
|
||
double get glarePosition => |
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 add some documentation above this getter so that anyone can understand what this logic does.
lib/credit_card_widget.dart
Outdated
///initialize the animation controller | ||
controller = AnimationController( | ||
duration: widget.animationDuration, | ||
vsync: this, | ||
); | ||
|
||
controller.addStatusListener((AnimationStatus status) { | ||
if (status == AnimationStatus.forward) { |
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 condition can be simplified like this
isAnimating = status == AnimationStatus.forward || status == AnimationStatus.reverse;
@@ -109,6 +120,36 @@ import 'package:flutter_credit_card/flutter_credit_card.dart'; | |||
), | |||
``` | |||
|
|||
*Floating Card* |
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.
I think a row of check boxes with indicating supported platform should also be added
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.
I believe that would be counter intuitive as we are currently not supporting only mobile web.
5d19797
to
ee2e2d5
Compare
example/lib/main.dart
Outdated
color: bgColor, | ||
image: DecorationImage( | ||
image: ExactAssetImage( | ||
isLightTheme ? 'assets/bg-white.png' : 'assets/bg.png', |
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.
convert assets/bg.png > assets/bg-dark.png and assets/bg-white.png > assets/bg-light.png
example/lib/main.dart
Outdated
enabledBorder: border, | ||
labelText: 'Card Holder', | ||
CreditCardWidget( | ||
floatConfig: !useFloatingAnimation |
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.
instead o negation sign switch the condition and code
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.
I would suggest to use a flag to enable floating animation. like enableFloatingCard and use floatConfig as configuration only if it is null then use default values
example/lib/main.dart
Outdated
? null | ||
: FloatConfig( | ||
isGlareEnabled: true, | ||
shadowConfig: FloatShadowConfig.preset(), |
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.
consider null as default value instead of FloatShadowConfig.preset()
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.
That will by default disable the shadow effect. Can you kindly confirm if that is okay?
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.
so the idea over here is to have a flag to enable or disable shadow which is by default enabled and the purpose of config will only be to change the paramteres of shadow.
fd3042a
to
9b421db
Compare
9b421db
to
751e11d
Compare
import 'floating_animation/floating_event.dart'; | ||
import 'flutter_credit_card_platform_interface.dart'; | ||
|
||
const String _methodChannelName = 'com.simform.flutter_credit_card'; |
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.
Lets added channel names and method name to constant file.
AppConstants.defaultGlareColor.withOpacity(0.1), | ||
AppConstants.defaultGlareColor.withOpacity(0.07), | ||
AppConstants.defaultGlareColor.withOpacity(0.05), | ||
]; |
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.
Instead of static member can we create a getter for this?
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.
Creating getter would cause creation of the object in question again and again on every usage of it. I believe using this way would save that case.
android/src/main/kotlin/com/simform/flutter_credit_card/FlutterCreditCardPlugin.kt
Show resolved
Hide resolved
android/src/main/kotlin/com/simform/flutter_credit_card/FlutterCreditCardPlugin.kt
Show resolved
Hide resolved
4912cef
to
1a42482
Compare
android/build.gradle
Outdated
|
||
dependencies { | ||
testImplementation 'org.jetbrains.kotlin:kotlin-test' | ||
testImplementation 'org.mockito:mockito-core:5.0.0' |
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.
I guess we can remove these dependencies
ce48ed9
to
4d7bc9e
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.
CHANGELOG.md
Outdated
@@ -1,6 +1,8 @@ | |||
# [3.0.8](https://github.com/SimformSolutionsPvtLtd/flutter_credit_card/tree/3.0.8) (Unreleased) |
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 major version here
4d7bc9e
to
5d1935a
Compare
pubspec.yaml
Outdated
@@ -1,22 +1,36 @@ | |||
name: flutter_credit_card | |||
description: A Credit Card widget package, support entering card details, card flip animation. | |||
description: A Credit Card widget package with support of entering card details, and animations like card flip | |||
and float. | |||
version: 3.0.8 |
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.
^^
- Added support for light and dark theme in example app. - Added preview for floating card animation with theme change in README.md. - Fixed github action workflow status url in README.md. - Added missing parameters in CreditCardWidget and CardCardForm in README.md.
5d1935a
to
38a3a70
Compare