LBRYCommunity/lbry-desktop

[redesign] even moar fixes #1270

Merged

neb-b merged 10 commits from redesign-fixes into master

2018-04-04 19:20:02 +02:00

neb-b commented

2018-04-04 01:21:13 +02:00

(Migrated from github.com)

Fixes:
#1218
#1219
#1226
#1227
#1263
https://github.com/lbryio/lbry-app/issues/1256
https://github.com/lbryio/lbry-app/issues/1223
https://github.com/lbryio/lbry-app/issues/1210

Fixes: #1218 #1219 #1226 #1227 #1263 https://github.com/lbryio/lbry-app/issues/1256 https://github.com/lbryio/lbry-app/issues/1223 https://github.com/lbryio/lbry-app/issues/1210

neb-b (Migrated from github.com) reviewed

2018-04-04 18:36:01 +02:00

src/renderer/component/publishForm/view.jsx

					
				@ -150,4 +105,4 @@

				    if (!name) {

				      const parsedFileName = fileName.replace(regexInvalidURI, '');

				      const uri = this.getNewUri(parsedFileName, channel);

				      newFileParams.name = parsedFileName;

Just changed the order of these. Lint was complaining

Just changed the order of these. Lint was complaining

liamcardenas (Migrated from github.com) approved these changes

2018-04-04 19:05:21 +02:00

liamcardenas (Migrated from github.com) left a comment

feel free to merge this, everything looks great to me. I left some comments, but nothing substantive

feel free to merge this, everything looks great to me. I left some comments, but nothing substantive

src/renderer/component/publishForm/view.jsx

					
				@ -154,3 +109,4 @@

				      newFileParams.uri = uri;

				    }

				    updatePublishForm(newFileParams);

My comment here is same as the other comment below about a similar line

My comment here is same as the other comment below about a similar line

src/renderer/redux/actions/publish.js

given your change elsewhere that added !channel || ... to it, does that need to be done here as well?

given your change elsewhere that added `!channel || ...` to it, does that need to be done here as well?

src/renderer/redux/actions/publish.js

not trying to make frivolous comments, but maybe this should be named sources and not source

not trying to make frivolous comments, but maybe this should be named `sources` and not `source`

liamcardenas (Migrated from github.com) reviewed

2018-04-04 19:07:14 +02:00

src/renderer/component/publishForm/view.jsx

					
				@ -154,3 +109,4 @@

				      newFileParams.uri = uri;

				    }

				    updatePublishForm(newFileParams);

actually, the comments is above in GitHub:

"given your change elsewhere that added !channel || ... to it, does that need to be done here as well?"

actually, the comments is above in GitHub: "given your change elsewhere that added !channel || ... to it, does that need to be done here as well?"

neb-b (Migrated from github.com) reviewed

2018-04-04 19:14:21 +02:00

src/renderer/redux/actions/publish.js

This is fine, because channel will definitely exist at this point. The other place there was a chance channel could be undefined

This is fine, because channel will definitely exist at this point. The other place there was a chance channel could be `undefined`

Sign in to join this conversation.

No reviewers

No labels

area: discovery

area: installer

area: livestream

area: performance

area: subscriptions

feature request

first-timers-only

good first issue

merge when green

needs: exploration

needs: grooming

needs: priority

needs: tech design

priority: blocker

priority: medium

protocol dependent

type: discussion

type: improvement

type: new feature

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-desktop!1270

No description provided.