Channel creation and editing #36

2019-09-02T14:05:47+02:00

akinwale commented

2019-09-02 14:05:47 +02:00

(Migrated from github.com)

No description provided.

neb-b (Migrated from github.com) reviewed 2019-09-02 14:05:47 +02:00

kauffj (Migrated from github.com) reviewed 2019-09-04 00:17:27 +02:00

src/component/claimList/view.js

					
				@ -208,0 +208,4 @@

				    if (uris) {

				      uris = uris.filter(uri => uri && uri.length > 0);

				    }

kauffj (Migrated from github.com) commented

2019-09-04 00:11:57 +02:00

(update to 0.40?)

kauffj (Migrated from github.com) commented

2019-09-04 00:12:49 +02:00

Aside from updating to 0.40, I think it is a good idea to comment code like this so that others (including future you, which is a different person than now you):

understand why it is there
understand when it can be removed

Aside from updating to 0.40, I think it is a good idea to comment code like this so that others (including future you, which is a different person than now you): - understand why it is there - understand when it can be removed

src/utils/helper.js

					
				@ -259,0 +324,4 @@

				    })

				    .catch(err => {

				      if (failure) {

				        failure(err.message ? err.message : 'The image failed to upload.');

kauffj (Migrated from github.com) commented

2019-09-04 00:17:19 +02:00

do we know anything about the user at the time this is happening? any ability to get a partial channel name or something else in here aside from pure random?

src/utils/helper.js

kauffj (Migrated from github.com) commented

2019-09-04 00:15:01 +02:00

this makes me uncomfortable, could a real file inspection be done when the file extension is missing?

kauffj (Migrated from github.com) commented

2019-09-04 00:16:09 +02:00

(much less importantly and similar to a previous comment, you can save a check by doing the trim at assignment)

let fileExt = fileName.indexOf('.') > -1 ? fileName.substring(fileName.lastIndexOf('.') + 1).trim() : '';

If (fileExt) { ...

(much less importantly and similar to a previous comment, you can save a check by doing the trim at assignment) ``` let fileExt = fileName.indexOf('.') > -1 ? fileName.substring(fileName.lastIndexOf('.') + 1).trim() : ''; If (fileExt) { ... ```

kauffj (Migrated from github.com) commented

2019-09-04 00:16:35 +02:00

also get that 0 out of there 😛

also get that 0 out of there :stuck_out_tongue:

kauffj (Migrated from github.com) reviewed 2019-09-12 00:26:39 +02:00

src/page/file/view.js

kauffj (Migrated from github.com) commented

2019-09-12 00:21:10 +02:00

parseURI returns isChannel

`parseURI` returns `isChannel`

src/page/file/view.js

kauffj (Migrated from github.com) commented

2019-09-12 00:21:21 +02:00

i18n

src/page/file/view.js

kauffj (Migrated from github.com) commented

2019-09-12 00:21:26 +02:00

i18n

src/redux/reducers/drawer.js

					
				@ -20,0 +27,4 @@

				  if (lastRoute === Constants.DRAWER_ROUTE_PUBLISH_FORM && routeName === Constants.DRAWER_ROUTE_PUBLISH) {

				    canPushStack = false;

				  }

kauffj (Migrated from github.com) commented

2019-09-12 00:22:38 +02:00

I'll be honest that I don't really understand what the above is doing, but it smells hackish 👃

I'll be honest that I don't really understand what the above is doing, but it smells hackish :nose:

src/utils/helper.js

					
				@ -171,3 +187,3 @@

				  dispatch(NavigationActions.back());

				  if (DrawerRoutes.indexOf(route) === -1 && isURIValid(route)) {

				  if (!DrawerRoutes.includes(route) && !InnerDrawerRoutes.includes(route) && isURIValid(route)) {

				    dispatchNavigateToUri(dispatch, nav, route, true);

kauffj (Migrated from github.com) commented

2019-09-12 00:25:13 +02:00

This also smells. I'm guessing all of this is to properly retain navigation on channel creation and publishing, especially in the cases where you might be auto-navigated/directed to those areas?

I suspect there's a design that's a little cleaner here. Ping me tomorrow and we can get on a call to discuss.

This also smells. I'm guessing all of this is to properly retain navigation on channel creation and publishing, especially in the cases where you might be auto-navigated/directed to those areas? I suspect there's a design that's a little cleaner here. Ping me tomorrow and we can get on a call to discuss.

src/utils/helper.js

kauffj (Migrated from github.com) commented

2019-09-12 00:26:08 +02:00

is err.message guaranteed to be set?

akinwale (Migrated from github.com) reviewed 2019-09-12 08:30:08 +02:00

src/utils/helper.js

					
				@ -171,3 +187,3 @@

				  dispatch(NavigationActions.back());

				  if (DrawerRoutes.indexOf(route) === -1 && isURIValid(route)) {

				  if (!DrawerRoutes.includes(route) && !InnerDrawerRoutes.includes(route) && isURIValid(route)) {

				    dispatchNavigateToUri(dispatch, nav, route, true);

akinwale (Migrated from github.com) commented

2019-09-12 08:30:08 +02:00

The drawer stack is a LIFO structure used for maintaining the navigation state, since the drawer navigator doesn't properly support back navigation out of the box.

In retrospect, the ideal solution here would be to use a nested StackNavigator for these pages (list / selector and the form), but I had already started down this road and didn't feel like separating into multiple components).

The drawer stack is a LIFO structure used for maintaining the navigation state, since the drawer navigator doesn't properly support back navigation out of the box. In retrospect, the ideal solution here would be to use a nested StackNavigator for these pages (list / selector and the form), but I had already started down this road and didn't feel like separating into multiple components).

akinwale (Migrated from github.com) reviewed 2019-09-12 08:30:15 +02:00

src/utils/helper.js

akinwale (Migrated from github.com) commented

2019-09-12 08:30:15 +02:00

I would hope so!

akinwale (Migrated from github.com) reviewed 2019-09-12 08:38:39 +02:00

src/redux/reducers/drawer.js

					
				@ -20,0 +27,4 @@

				  if (lastRoute === Constants.DRAWER_ROUTE_PUBLISH_FORM && routeName === Constants.DRAWER_ROUTE_PUBLISH) {

				    canPushStack = false;

				  }

akinwale (Migrated from github.com) commented

2019-09-12 08:38:39 +02:00

On the publish page, we have two screens: the gallery selector and the publish form.
On the channels page, we have the channel list and the channel editor form.
This is essentially checking if we're on one of the pages (or a related sub screen) before pushing into the stack.

On the publish page, we have two screens: the gallery selector and the publish form. On the channels page, we have the channel list and the channel editor form. This is essentially checking if we're on one of the pages (or a related sub screen) before pushing into the stack.

Sign in to join this conversation.

No reviewers

neb-b

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: LBRYCommunity/lbry-react-native#36