LBRYCommunity/lbry-android
1
0
Fork 0
Code Issues 155 Pull requests 6 Projects Releases 84 Packages Wiki Activity

prevent an error upon pressing the back button on the splash screen #212

Merged
akinwale merged 1 commit from splash-screen-back into master 2018-07-31 16:39:30 +02:00
Conversation 0 Commits 1 Files changed 1 +8 -6
1 changed files with 8 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

14
app/src/component/AppNavigator.js
View file

@ -128,12 +128,14 @@ class AppWithNavigationState extends React.Component {
const { dispatch, nav } = this.props;
// There should be a better way to check this
if (nav.routes.length > 0) {
const subRoutes = nav.routes[0].routes[0].routes;
const lastRoute = subRoutes[subRoutes.length - 1];
if (nav.routes[0].routes[0].index > 0 &&
['About', 'Settings'].indexOf(lastRoute.key) > -1) {
dispatch(NavigationActions.back());
return true;
if (nav.routes[0].routes && nav.routes[0].routes.length > 0) {
const subRoutes = nav.routes[0].routes[0].routes;
const lastRoute = subRoutes[subRoutes.length - 1];
if (nav.routes[0].routes[0].index > 0 &&
['About', 'Settings'].indexOf(lastRoute.key) > -1) {
dispatch(NavigationActions.back());
return true;
}
}
if (nav.routes[0].routeName === 'Main') {
neb-b commented 2018-07-24 17:06:48 +02:00 (Migrated from github.com)
Review
Copy link

nav.routes[0].routes[0].index > 0

what is .index?

`nav.routes[0].routes[0].index > 0` what is `.index`?
akinwale commented 2018-07-27 07:17:16 +02:00 (Migrated from github.com)
Review
Copy link

@seanyesmunt It's the index of the current active route in the navigation state. See https://reactnavigation.org/docs/en/stack-actions.html.

@seanyesmunt It's the index of the current active route in the navigation state. See https://reactnavigation.org/docs/en/stack-actions.html.
if (nav.routes[0].routes[0].routes[0].index > 0) {
skhameneh commented 2018-07-27 08:13:10 +02:00 (Migrated from github.com)
Review
Copy link

This seems very volatile to changes, I'll take a closer look tomorrow.

This seems very volatile to changes, I'll take a closer look tomorrow.
akinwale commented 2018-07-30 12:23:19 +02:00 (Migrated from github.com)
Review
Copy link

Yes, I agree. However, since we have a fixed navigation structure which is unlikely to change often, I think it's a passable solution until we can come up with a better approach. See f8af04ba4b/app/src/component/AppNavigator.js (L129)

Yes, I agree. However, since we have a fixed navigation structure which is unlikely to change often, I think it's a passable solution until we can come up with a better approach. See https://github.com/lbryio/lbry-android/blob/f8af04ba4b20162d714501acdcf59dcb829f9f41/app/src/component/AppNavigator.js#L129
kauffj commented 2018-07-30 16:23:15 +02:00 (Migrated from github.com)
Review
Copy link

Setting aside the specific code choices made here, I wanted to comment on the more general situation of how we handle shipping mediocre/bad temporary solutions (which is part of normal development and not something that has to be avoided 100%).

If we're writing code that we know is subpar pending another change or an issue, we should:

  1. Make sure that issue is filed ("Proper Routing Architecture" or whatever the more fundamental issue here is)
  2. Make sure that issue is linked in code and that the issue links to the code

Coming back to this specific choice, if we must do things this way, it could make sense to hoist the declaration up to the top of the file to add clarity about it's existence.

Setting aside the specific code choices made here, I wanted to comment on the more general situation of how we handle shipping mediocre/bad temporary solutions (which is part of normal development and not something that has to be avoided 100%). If we're writing code that we know is subpar pending another change or an issue, we should: 1) Make sure that issue is filed ("Proper Routing Architecture" or whatever the more fundamental issue here is) 2) Make sure that issue is linked in code and that the issue links to the code Coming back to this specific choice, if we must do things this way, it could make sense to hoist the declaration up to the top of the file to add clarity about it's existence.