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

2018-07-24T15:39:49+02:00

akinwale commented

2018-07-24 15:39:49 +02:00

(Migrated from github.com)

No description provided.

neb-b (Migrated from github.com) reviewed 2018-07-24 17:07:01 +02:00

neb-b (Migrated from github.com) left a comment

One comment

app/src/component/AppNavigator.js

					
				@ -137,2 +137,4 @@

				            return true;

				          }

				        }

				        if (nav.routes[0].routeName === 'Main') {

neb-b (Migrated from github.com) commented

2018-07-24 17:06:48 +02:00

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

what is .index?

`nav.routes[0].routes[0].index > 0` what is `.index`?

akinwale (Migrated from github.com) reviewed 2018-07-27 07:17:16 +02:00

app/src/component/AppNavigator.js

					
				@ -137,2 +137,4 @@

				            return true;

				          }

				        }

				        if (nav.routes[0].routeName === 'Main') {

akinwale (Migrated from github.com) commented

2018-07-27 07:17:16 +02:00

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

skhameneh (Migrated from github.com) reviewed 2018-07-27 17:05:51 +02:00

app/src/component/AppNavigator.js

					
				@ -137,3 +138,4 @@

				          }

				        }

				        if (nav.routes[0].routeName === 'Main') {

				          if (nav.routes[0].routes[0].routes[0].index > 0) {

skhameneh (Migrated from github.com) commented

2018-07-27 08:13:10 +02:00

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

akinwale (Migrated from github.com) reviewed 2018-07-30 12:23:19 +02:00

app/src/component/AppNavigator.js

					
				@ -137,3 +138,4 @@

				          }

				        }

				        if (nav.routes[0].routeName === 'Main') {

				          if (nav.routes[0].routes[0].routes[0].index > 0) {

akinwale (Migrated from github.com) commented

2018-07-30 12:23:19 +02:00

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 (Migrated from github.com) reviewed 2018-07-30 16:23:15 +02:00

app/src/component/AppNavigator.js

					
				@ -137,3 +138,4 @@

				          }

				        }

				        if (nav.routes[0].routeName === 'Main') {

				          if (nav.routes[0].routes[0].routes[0].index > 0) {

kauffj (Migrated from github.com) commented

2018-07-30 16:23:15 +02:00

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:

Make sure that issue is filed ("Proper Routing Architecture" or whatever the more fundamental issue here is)
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.

Sign in to join this conversation.

No reviewers