-
Notifications
You must be signed in to change notification settings - Fork 92
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
Issue 42 #229
Issue 42 #229
Conversation
Hey, @himanshusharma89, kindly review this Pr. The push notification is working and rebasing with the master branch is also done! |
@@ -0,0 +1,8 @@ | |||
## This file must *NOT* be checked into Version Control Systems, |
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.
Don't add the auto-generated files in the PR.
@@ -6,7 +6,7 @@ | |||
additional functionality it is fine to subclass or reimplement | |||
FlutterApplication and put your custom class here. --> | |||
<application | |||
android:name="io.flutter.app.FlutterApplication" | |||
android:name=".Application" |
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 Flutter version is already greater than 1.12, so no need to integrate FCM in such a way, revert back to io.Flutter.app.FlutterApplcation
@@ -0,0 +1,18 @@ | |||
package tech.himanshusharma.relicbazaar |
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 Flutter version is already greater than 1.12, so no need to integrate FCM in such a way.
@@ -6,7 +6,7 @@ | |||
additional functionality it is fine to subclass or reimplement | |||
FlutterApplication and put your custom class here. --> | |||
<application | |||
android:name="io.flutter.app.FlutterApplication" | |||
android:name=".Application" | |||
android:label="retro_shopping" |
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.
Update the below android:label to Relic Bazaar
@@ -33,15 +33,15 @@ class RemoteConfigService { | |||
try { | |||
await _remoteConfig.setDefaults(defaults); | |||
await _fetchAndActivate(); | |||
} on FetchThrottledException catch (e) { | |||
} on Exception catch (e) { |
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.
Revert back to FetchThrottledException, as every other exception will be caught by the cath block below.
debugPrint('Remote config fetch throttled : $e'); | ||
} catch (e) { | ||
debugPrint('Unable to fecth'); | ||
} | ||
} | ||
|
||
Future<void> _fetchAndActivate() async { | ||
await _remoteConfig.fetch(expiration: const Duration(hours: 4)); | ||
await _remoteConfig.activateFetched(); | ||
await _remoteConfig.fetch(); |
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 _remoteConfig.fetchAndActivate(), instead of 2 different command.
@@ -1,5 +1,5 @@ | |||
import 'package:advance_pdf_viewer/advance_pdf_viewer.dart'; | |||
import 'package:flutter/material.dart'; |
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 have done a lot of changes in this file, attach a video of this new view in the comments
firebase_remote_config: ^0.6.0 | ||
firebase_core: any | ||
google_fonts: ^1.1.0 | ||
firebase_remote_config: ^0.10.0-dev.2 |
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.
Don't use pre-release versions.
firebase_auth: 0.20.1 | ||
native_pdf_view: ^4.0.1 | ||
firebase_auth: ^1.1.3 | ||
http_parser: ^4.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.
http_parser is not being used anywhere, so remove it.
@@ -36,7 +63,45 @@ Future<void> main() async { | |||
); | |||
} | |||
|
|||
class MyApp extends StatelessWidget { | |||
class MyApp extends StatefulWidget { |
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.
Don't convert the MyApp to stateful, shift the token generation and initialization of push notification in dashboard.dart
Related Issue
Proposed Changes
Additional Information
Checklist
ScreenVideo
relic_bazar_push_notif.mp4