-
-
Notifications
You must be signed in to change notification settings - Fork 287
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: added deeplinking for the sign up page #4169 #5332
Conversation
### What <!-- Added deep linking for the sign up page--> - <!-- Changed x to achieve y --> <!-- Please name your pull request following this scheme: `type: What you did` this allows us to automatically generate the changelog Following `type`s are allowed: - `feat`, for Features - `fix`, for Bug Fixes - `docs`, for Documentation - `ci`, for Automation - `refactor`, for code Refactoring - `chore`, for Miscellaneous things --> ### Screenshot <!-- Insert a screenshot to provide visual record of your changes, if visible --> ### Fixes bug(s) <!-- change by appropriate issues. --> <!-- Please use a linking keyword https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword --> - Fixes: <!-- openfoodfacts#1 Note: you can also use Closes: --> ### Part of - openfoodfacts#525 <!-- Add the most granular issue possible -->
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.
feat: Added deeplinking for sign up page & resest user preferences for onboarding once sign up is finished
Thanks for your PR @jnnabugwu. The idea in the app is to catch public URLs (eg: /signup) and redirect to internal pages (eg: /_signup) |
I'll fix this. |
Its fixed |
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.
For the deep link, it now seems to be OK 👌
However storing preferences is unnecessary in this screen
@@ -46,8 +47,20 @@ class _SignUpPageState extends State<SignUpPage> with TraceableClientMixin { | |||
bool _subscribe = false; | |||
bool _disagreed = false; | |||
|
|||
late UserPreferences _userPreferences; |
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.
UserPreferences
is only used after a successful registration.
For all other scenarios, it's totally unnecessary.
Instead, you just need to access the preferences when you need it.
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.
okay I see what you are saying
@override | ||
void initState() { | ||
super.initState(); | ||
getUserPreferences(); |
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.
Be careful about formatting your 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.
What do you mean by 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.
Launch the dartfmt
command
https://docs.flutter.dev/tools/formatting
@@ -411,6 +425,8 @@ class _SignUpPageState extends State<SignUpPage> with TraceableClientMixin { | |||
if (!mounted) { | |||
return; | |||
} | |||
debugPrint('Reseting Onboarding'); | |||
_userPreferences.resetOnboarding(); |
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 get the preferences here
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.
Will do
@@ -411,6 +425,8 @@ class _SignUpPageState extends State<SignUpPage> with TraceableClientMixin { | |||
if (!mounted) { | |||
return; | |||
} | |||
debugPrint('Reseting Onboarding'); |
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.
For debugging purposes, you have our custom Log
class
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's OK for me, thanks for your contribution!
### What
Added deep linking for the sign-up page. Also, reset the users onboarding settings so that it shows them onboarding right after they sign up.
To test the screen before releasing it, a deep link is now available: https://fr.openfoodfacts.org/_signup/
Screenshot
8mb.video-BWs-td0VSE0s.mp4
Fixes bug(s)
Part of