🪆✈️✈️✈️ react native navigation ✈️✈️✈️🪆 #12141
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Jenkins BuildsClick to see older builds (145)
|
|
The error is: All I did is followed these instructions: But it failed with: And then I redid the same seps but for This seems to be a bigger issue. |
|
So here's a patch that fixes the build: https://gist.github.com/jakubgs/f03d16295a93377a1adb643e8e0c4f99 But the issue with that is that these dependencies disappear when I run But at least this patch should unblock you to work on this while I research a bit more. |
|
This is interesting: Looks like our AWK script is not including this dependency in the output from |
|
This is the full output from: https://gist.github.com/jakubgs/8e7dd5d226170415e99708463731311c Our AWK script looks for targets from that command that are But the target names for Which means the proper fix would be to add |
jakubgs
added a commit
that referenced
this pull request
May 25, 2021
This fixes an issue first detected in: #12141 Which resulted in dependencies being ignored for `react-native-nagivation`. Signed-off-by: Jakub Sokołowski <[email protected]>
|
Whoops, pushed the fix to |
jakubgs
force-pushed
the
feature/react-native-navigation
branch
from
623ba16 to
db42dcf
Compare
flexsurfer
force-pushed
the
feature/react-native-navigation
branch
from
db42dcf to
ac02fee
Compare
flexsurfer
force-pushed
the
feature/react-native-navigation
branch
3 times, most recently
from
e3cf969 to
8e65a0c
Compare
|
a couple of issues I noticed on my side:
Those are the ones I found so far, but the performance difference is astonishing, especially on low powered devices. Amazing work, really impressive! |
|
thanks
fixed |
1% of end-end tests have passedFailed tests (69)Click to expand
Passed tests (1) |
flexsurfer
force-pushed
the
feature/react-native-navigation
branch
from
4c0d919 to
6e8d8d7
Compare
flexsurfer
force-pushed
the
feature/react-native-navigation
branch
from
6e8d8d7 to
4e9f429
Compare
|
Issue1
Issue2 |
1% of end-end tests have passedFailed tests (69)Click to expand
Passed tests (1) |
@flexsurfer can we please fix this with a 1px top border? similar like we have for top bars |
|
@errorists in this PR along performance improvements there is one button replacement happened. And need you opinion whether it's better now of we have to not change it:
Do you think it makes sense to keep |
|
I'm thinking the one on the left, but it's not a strong difference I'm fine with keeping the |
We need to add non-transparent background when navigating between views because navigation doesn't look "smooth": 12fdff.mov |
|
Issue7 Double tap on Browser Tab button has no effect in respect to return to Browser main tab view (all other tab buttons working OK) as designed, there is no main tab view for browser tab |
|
Issue4 After user is blocked from public chat - when I return back then there is an “Unknown” chat view unable to replicate |
|
Issue9 There is no three-word signing phrase pop-up when opening the Wallet tab for the first time (however it appears when I try to send a transaction) this looks like a feature :) |
shivekkhurana
approved these changes
Jun 9, 2021
There was a problem hiding this comment.
This is an extraordinary piece of work. It is the second time since I've been here that Andrey has pulled a feat like this (earlier, he refactored the events system).
I have left many comments. The ones marked with [SELF NOTE] are for me to check and resolve. There are a few small issues that I've pointed out.
| (get-in views/screens [screen :options])))) | ||
|
|
||
| ;;TODO problem here is that we have two places for screens, here and in screens ns, and we have handler in navigate | ||
| (defn roots [] |
There was a problem hiding this comment.
Dedicated roots map is helpful. Why is having 2 ns for screens a problem?
| [react/view {:flex 1 :background-color colors/white} | ||
| [toolbar-view name] | ||
| [:<> | ||
| [topbar/topbar |
There was a problem hiding this comment.
Should this be replaced with native topbar?
grou-chat-e.mov |
flexsurfer
force-pushed
the
feature/react-native-navigation
branch
from
7236c30 to
4a723c3
Compare
|
@Serhy keycard fixed |
Reproduction:
|
|
@Serhy fixed |
flexsurfer
force-pushed
the
feature/react-native-navigation
branch
from
2d29613 to
b82a31b
Compare
49% of end-end tests have passedFailed tests (36)Click to expand
Passed tests (34)Click to expand |
100% of end-end tests have passedPassed tests (3)Click to expand
|
63% of end-end tests have passedFailed tests (26)Click to expand
Passed tests (44)Click to expand |
|
Those failed e2e tests will be fixed separately as they are mostly e2e specific (on real devices it looks good). |
flexsurfer
force-pushed
the
feature/react-native-navigation
branch
from
018e7bb to
a2655a1
Compare
Signed-off-by: andrey <[email protected]>
flexsurfer
force-pushed
the
feature/react-native-navigation
branch
from
a2655a1 to
5f719ac
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
react native navigation
fixes: #9641
fixes: #11556
This PR is replacing https://github.com/react-navigation with https://github.com/wix/react-native-navigation
Why?
react-navigation is written with javascript, and uses js thread for actions and animations, also it uses one root for the entire application
react-native-navigation is native, so all elements (buttons, bottom bar .. ) animations and actions are native, js thread isn't used, each screen has its own root, modals, and sheets rendered in a separate container, so they don't affect the main screen
so react-native-navigation should be more performant and feels more native
but because all controls are native we can tune their appearance only with a fixed number of properties, which is less flexible
Also react-native-navigation controls statusBar and navigationBar (android) styles
New deps:
"react-native-navigation": "^7.13.0"
Removed deps:
"@react-navigation/bottom-tabs": "^5.8.0",
"@react-navigation/native": "^5.7.3",
"@react-navigation/stack": "^5.9.0",
"react-native-navigation-bar-color": "^2.0.1"
"react-native-screens": "^2.10.1"
major issues:
issues:
TODO:
Next:
Latest build to try:
| ✔️ | 3a80501 | #45 | 2021-06-10 11:41:56 | ~13 min |
android| 📦apk📲|| ✔️ | 3a80501 | #44 | 2021-06-10 11:41:58 | ~13 min |
android-e2e| 📦apk📲|| ✔️ | 3a80501 | #43 | 2021-06-10 11:43:24 | ~14 min |
ios| 📦ipa📲|