feat: onboarding transitions for new to status flow #16554
Conversation
Jenkins BuildsClick to see older builds (84)
|
briansztamfater
force-pushed
the
feat/new-to-status-transition
branch
from
dbcdec8
to
1f7848e
Compare
briansztamfater
force-pushed
the
feat/new-to-status-transition
branch
6 times, most recently
from
024ce86
to
505515d
Compare
| blur-show-fn (fn [] | ||
| (reanimated/animate opacity | ||
| 1 | ||
| transition-half-duration-ms) | ||
| (js/clearInterval @timer-interval) | ||
| (reset! timer-interval | ||
| (js/setInterval | ||
| (fn [] | ||
| (if (< @blur-amount max-blur-amount) | ||
| (swap! blur-amount + 1) | ||
| (js/clearInterval @timer-interval))) | ||
| (/ constants/onboarding-modal-animation-duration | ||
| max-blur-amount | ||
| 2)))) |
There was a problem hiding this comment.
We cannot animate blur-amount, so we use a ratom and a timer to update blur-amount value + animate opacity to achieve some smoothness.
We use this animation while pushing a screen with transparent background to achieve an effect of blurring the background while the new content is pushed.
briansztamfater
changed the title
briansztamfater
force-pushed
the
feat/new-to-status-transition
branch
from
f8ce682
to
d907aa6
Compare
briansztamfater
requested review from
alwx,
ajayesivan,
smohamedjavid,
J-Son89 and
siddarthkay
| @@ -6,9 +6,8 @@ | |||
|
|
|||
| (defn page-container | |||
| [insets] | |||
| {:flex 1 | |||
| :padding-top (:top insets) | |||
| :background-color colors/neutral-80-opa-80-blur}) | |||
There was a problem hiding this comment.
We don't need background colors anymore inside onboarding screens because we now use the screen behind as the background
| @@ -26,7 +26,7 @@ | |||
| {:events [:onboarding-2/profile-data-set]} | |||
| [{:keys [db]} onboarding-data] | |||
| {:db (update db :onboarding-2/profile merge onboarding-data) | |||
| :dispatch [:navigate-to :create-profile-password]}) | |||
| :dispatch [:navigate-to-within-stack [:create-profile-password :new-to-status]]}) | |||
There was a problem hiding this comment.
We now navigate within the :new-to-status stack that is inside a modal
| @@ -1,7 +1,8 @@ | |||
| (ns status-im2.contexts.onboarding.identifiers.style) | |||
|
|
|||
| (def page-container | |||
| {:flex 1}) | |||
| {:flex 1 | |||
| :overflow :hidden}) | |||
There was a problem hiding this comment.
Fix for a glitch when navigating to other screen and the carousel overflows the screen bounds
| (def new-to-status-modal-animations | ||
| {:showModal {:translationX {:from (:width (rn/get-window)) | ||
| :to 0 | ||
| :duration constants/onboarding-modal-animation-duration}} | ||
| :dismissModal {:translationX {:from 0 | ||
| :to (:width (rn/get-window)) | ||
| :duration constants/onboarding-modal-animation-duration}}}) |
There was a problem hiding this comment.
For :new-to-status screen we now use a modal, but change transition animations to behave like a normal push. This is because we need to preserve the screen behind as a background and the only way to do that is using a modal with :modalPresentationStyle option with :overCurrentContext as its value
src/status_im2/contexts/onboarding/common/navigation_bar/view.cljs
Outdated
Show resolved
Hide resolved
| (/ constants/onboarding-modal-animation-duration | ||
| max-blur-amount | ||
| 2))))] | ||
| (rn/use-effect |
There was a problem hiding this comment.
consider using reagent/with-let. cc @ulisesmac
There was a problem hiding this comment.
I thought the same at first sight, then I tried to modify the code to achieve it but it's not possible.
The functions @briansztamfater is passing to the atoms depend on a reanimated shared value, which is a hook. so, the problem is, the bindings in with-let are executed only once (when the component is mounted) and we'd need to create the opacity shared value there, but in React we must use the same amount of hooks in every re-render.
btw, I haven't shared the with-let explanation, if you want to read it:
https://github.com/reagent-project/reagent/blob/master/doc/CreatingReagentComponents.md#appendix-b---with-let-macro
Just to be clear enough:
If we declare opacity in reagent/with-let, the hook will be created in the first render, then for the second render, that hook will not be executed, so React will complain about it and it'll throw an exception saying somehting like
"Rendered fewer hooks than expected. This may be caused by an accidental early return statement in React Hooks"
There was a problem hiding this comment.
@briansztamfater - We have an issue. When taps on the back button in the :welcome screen, it takes the user back to :enable-notification screen with the help of :init-root.
Because of :init-root the blurred background is not displayed.
I'm not sure. Maybe, we need to include the :welcome screen in the modal presentation to fix it.
But, we will have this same issue during syncing as once syncing is successful, we take the user to the :enable-notification screen.
@smohamedjavid Thanks for taking the time to test and finding this issue, I'll check it out and implement a solution :) |
✔️ status-mobile/prs/android-e2e/PR-16554#24 🔹 ~6 min 2 sec 🔹 fa4770f 🔹 📦 android-e2e package |
briansztamfater
force-pushed
the
feat/new-to-status-transition
branch
from
3c782c3
to
5d3e475
Compare
|
@smohamedjavid I've implemented a solution adding The rest of suggestions from reviewers were implemented as well :) |
Thanks @briansztamfater! Duly appreciate it! 😄 |
| :popGesture false | ||
| :modalPresentationStyle :overCurrentContext | ||
| :hardwareBackButton {:dismissModalOnPress false | ||
| :popStackOnPress false}} |
There was a problem hiding this comment.
Nit: for using kebab case wherever possible 😅
86% of end-end tests have passedFailed tests (5)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityOneDeviceMerged:
Passed tests (31)Click to expandClass TestActivityMultipleDevicePR:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
Class TestCommunityMultipleDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
|
|
Hey @briansztamfater, thanks for your work! ISSUE 1:
|
|
@qoqobolo Thanks for testing! Will rebase and take a look at the issue :) |
briansztamfater
force-pushed
the
feat/new-to-status-transition
branch
2 times, most recently
from
499407d
to
8a7ba6c
Compare
|
@qoqobolo issue should be fixed now! |
|
Thanks @briansztamfater! |
briansztamfater
force-pushed
the
feat/new-to-status-transition
branch
from
8a7ba6c
to
fa5a644
Compare
Signed-off-by: Brian Sztamfater <brian@status.im>
briansztamfater
force-pushed
the
feat/new-to-status-transition
branch
from
fa5a644
to
5a3c66d
Compare
fixes #15877
Summary
Implement transitions for new to status flow in onboarding
Results
iOS
onboardingtransitions.mp4
Android
onboardingtransitionsandroid.mp4
Platforms
Functional
Steps to test
Scenario 1: new installation
Scenario 2: at least 1 account already created
Create new profilestatus: ready