Skip to content
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

[$250] LOW: Investigate using native-stack on native platforms instead of stack #29948

Closed
mountiny opened this issue Oct 19, 2023 · 67 comments
Closed
Assignees
Labels
External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item. Reviewing Has a PR in review Task Weekly KSv2

Comments

@mountiny
Copy link
Contributor

mountiny commented Oct 19, 2023

Coming from here

When we have refactored the App navigation, we have chose to use the normal react-navigation stack as that gave us more control over animation/ transitions of pages on web. This was to also not complicate the MVP for the project with two types of stack.

The problem is that the normal stack does not lead to native feel of app page transitions in native device which makes the feel of the app worse as the page transitions are ever so slightly off compared to the other native apps you might be using on daily basis.

Its important to address this discrepancy and ideally introduce the native-stack for native iOS and Android platforms, as chat/ P2P users have high standards for native app quality.

cc @hannojg @adamgrzybowski

Issue OwnerCurrent Issue Owner: @
Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021866177344381113052
  • Upwork Job ID: 1866177344381113052
  • Last Price Increase: 2024-12-09
@hannojg
Copy link
Contributor

hannojg commented Oct 19, 2023

Yes, will help investigating this or next week 🤝!

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Weekly KSv2 labels Oct 19, 2023
@hannojg
Copy link
Contributor

hannojg commented Oct 19, 2023

PR is up:

@hannojg
Copy link
Contributor

hannojg commented Oct 24, 2023

We got two blockers at the minute:

@WoLewicki
Copy link
Contributor

@hannojg as for the second dot, the ios like do you think that this animation is what people want on Android to have the native feel? Maybe we could explore the other animations that are strictly taken from the Android XMLs?

Also, could you elaborate how does native-stack improve the performance of the app exactly? One thing I can think of is native swipe to go back on iOS which is controlled by the native code, but you still need to mount the previous screen on the native side when it happens, did you check how it performs when going back to reports with many many messages?

@hannojg
Copy link
Contributor

hannojg commented Oct 25, 2023

ios like do you think that this animation is what people want on Android to have the native feel

We have to double check the animation. Its called iOS like but I think it actually feels like the android default slide transition

Also, could you elaborate how does native-stack improve the performance of the app exactly

This is more about perceived performance and UX feeling. Right now you can tell by using the expensify app, that the transitions aren't the native transitions. So we want to have the native transitions so expensify feels like a native app.

you still need to mount the previous screen on the native side when it happens, did you check how it performs when going back to reports with many many messages?

Do I understand it correctly that you are suggesting that switching to native-stack could be a performance issue when doing the swipe back gesture?

@WoLewicki
Copy link
Contributor

Do I understand it correctly that you are suggesting that switching to native-stack could be a performance issue when doing the swipe back gesture?

We have the same abstraction in the stack navigator, where we control the mounting/unmounting the views by using activityState prop. Native-stack should not have worse performance, I'm just curious if it is fast enough no to be laggy in high traffic accounts 😅

@mountiny
Copy link
Contributor Author

we should test it out with high traffic accounts, the E2E tests once are built in like 2 weeks should also tell us more how it performs there

@muttmuure muttmuure moved this to MEDIUM in [#whatsnext] #quality Nov 13, 2023
@melvin-bot melvin-bot bot added Monthly KSv2 and removed Weekly KSv2 labels Nov 20, 2023
Copy link

melvin-bot bot commented Nov 20, 2023

This issue has not been updated in over 15 days. @hannojg, @mountiny eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

@mountiny
Copy link
Contributor Author

Keeping this monthly, its not high priority and still waiting on some issues to get resolved.

@muttmuure muttmuure moved this from MEDIUM to LOW in [#whatsnext] #quality Dec 11, 2023
Copy link

melvin-bot bot commented Jan 16, 2024

@hannojg, @mountiny, this Monthly task hasn't been acted upon in 6 weeks; closing.

If you disagree, feel encouraged to reopen it -- but pick your least important issue to close instead.

@mountiny
Copy link
Contributor Author

@hannojg How is this looking now with latesst ios, have they fixed anything?

@mountiny mountiny reopened this Jan 16, 2024
@mountiny mountiny removed the Monthly KSv2 label Jan 16, 2024
@melvin-bot melvin-bot bot added the Monthly KSv2 label Jan 17, 2024
@mountiny
Copy link
Contributor Author

mountiny commented Dec 3, 2024

This was merged and now its in production 🎉

@joekaufmanexpensify
Copy link
Contributor

Going to issue payment here in the next day!

@mountiny
Copy link
Contributor Author

mountiny commented Dec 9, 2024

PRs to pay out:

I would say $1000 to @ishpaul777 and $250 to @c3024 for their work on these

@joekaufmanexpensify
Copy link
Contributor

Got it. TY! Opening new upwork job so we can pay.

@joekaufmanexpensify joekaufmanexpensify added the External Added to denote the issue can be worked on by a contributor label Dec 9, 2024
@melvin-bot melvin-bot bot changed the title LOW: Investigate using native-stack on native platforms instead of stack [$250] LOW: Investigate using native-stack on native platforms instead of stack Dec 9, 2024
Copy link

melvin-bot bot commented Dec 9, 2024

Job added to Upwork: https://www.upwork.com/jobs/~021866177344381113052

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Dec 9, 2024
Copy link

melvin-bot bot commented Dec 9, 2024

Current assignee @ishpaul777 is eligible for the External assigner, not assigning anyone new.

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Dec 9, 2024
@joekaufmanexpensify joekaufmanexpensify added Weekly KSv2 and removed Daily KSv2 Help Wanted Apply this label when an issue is open to proposals by contributors labels Dec 9, 2024
@joekaufmanexpensify
Copy link
Contributor

Ah, actually noticed @c3024 is already being paid for their PR here. So we just need to pay @ishpaul777

@joekaufmanexpensify
Copy link
Contributor

joekaufmanexpensify commented Dec 9, 2024

Payment summary is $1,250 to @ishpaul777 for C+ review via upwork.

@joekaufmanexpensify
Copy link
Contributor

@ishpaul777 offer sent in Upwork!

@ishpaul777
Copy link
Contributor

@mountiny I also reviewed this PR: #49936. Additionally, I believe the effort and time involved in this were significant. Could we consider adjusting the bounty slightly, perhaps to $1500 for all three PRs I reviewed?

@mountiny
Copy link
Contributor Author

@ishpaul777 ah nice, thanks for the link. Let's increase to $1250, including that PR. Thanks!

@joekaufmanexpensify
Copy link
Contributor

Sounds good! Adjusted payment summary. @ishpaul777 please accept the offer I sent. I will pay the extra $250 as a bonus.

@ishpaul777
Copy link
Contributor

Sounds good! Accepted offer!

@joekaufmanexpensify
Copy link
Contributor

@ishpaul777 $1,250 sent and contract ended!

@joekaufmanexpensify
Copy link
Contributor

Upwork job closed.

@joekaufmanexpensify
Copy link
Contributor

All set, thanks everyone!

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Weekly KSv2 labels Dec 13, 2024
Copy link

melvin-bot bot commented Dec 18, 2024

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item. Reviewing Has a PR in review Task Weekly KSv2
Projects
Development

No branches or pull requests

10 participants