RC fixes #2473

2019-05-10T16:54:15+02:00

neb-b commented

2019-05-10 16:54:15 +02:00

(Migrated from github.com)

Changes

Use port 9090 for webpack so internal apis can use it's default port
Add back support for unlocking supports
- Dependent on https://github.com/lbryio/lbry-redux/pull/144
Fix phone collection modal closing on success
Fix navigation away from error page in electron
- also maintains navigation state, so you can still go back after
Style consitency cleanup

#### Changes - Use port 9090 for webpack so internal apis can use it's default port - Add back support for unlocking supports - Dependent on https://github.com/lbryio/lbry-redux/pull/144 - Fix phone collection modal closing on success - Fix navigation away from error page in electron - also maintains navigation state, so you can still go back after - Style consitency cleanup

kauffj (Migrated from github.com) requested changes 2019-05-14 00:26:44 +02:00

kauffj (Migrated from github.com) left a comment

Some small changes requested

src/platforms/electron/devServer.js

kauffj (Migrated from github.com) commented

2019-05-14 00:19:36 +02:00

It looks like 9090 appears in several places in the code. Since we have to DRY break for pragmatic reasons here, the best way to handle this is:

Note in a comment by the primary definition (presumably webpack) where all secondary usages occur
Reference that devs should look at the primary definition in all secondary usages

It looks like 9090 appears in several places in the code. Since we have to DRY break for pragmatic reasons here, the best way to handle this is: - Note in a comment by the primary definition (presumably webpack) where all secondary usages occur - Reference that devs should look at the primary definition in all secondary usages

src/platforms/electron/devServer.js

kauffj (Migrated from github.com) commented

2019-05-14 00:19:53 +02:00

inject WEBPACK_PORT into string

src/ui/component/channelThumbnail/view.jsx

kauffj (Migrated from github.com) commented

2019-05-14 00:21:04 +02:00

I believe that thumbnailColorClass += (initializer % 15) or something close to this is equivalent to the entire switch

I believe that `thumbnailColorClass += (initializer % 15)` or something close to this is equivalent to the entire `switch`

src/ui/component/transactionList/view.jsx

kauffj (Migrated from github.com) commented

2019-05-14 00:22:52 +02:00

I believe you can have multiple supports in a single transaction. Should supports also be indexed by outpoint rather than txid?

src/ui/scss/component/_channel.scss

					
				@ -3,3 +3,3 @@

				.channel__cover {

				.channel-cover {

				  background-image: linear-gradient(to right, $lbry-indigo-4, $lbry-cyan-5 80%);

kauffj (Migrated from github.com) commented

2019-05-14 00:24:14 +02:00

thank you

neb-b (Migrated from github.com) reviewed 2019-05-14 04:42:52 +02:00

src/ui/component/transactionList/view.jsx

neb-b (Migrated from github.com) commented

2019-05-14 04:42:51 +02:00

~~Good point. Yeah, we probably should use it. Is this a blocker? RC testing has been using it. I can update after the release~~

decided to do it now https://github.com/lbryio/lbry-redux/pull/145

~Good point. Yeah, we probably should use it. Is this a blocker? RC testing has been using it. I can update after the release~ decided to do it now https://github.com/lbryio/lbry-redux/pull/145

neb-b (Migrated from github.com) reviewed 2019-05-14 07:20:55 +02:00

src/ui/component/channelThumbnail/view.jsx

neb-b (Migrated from github.com) commented

2019-05-14 07:20:54 +02:00

Yep. initializer % 4

Yep. `initializer % 4`

kauffj (Migrated from github.com) approved these changes 2019-05-14 19:27:37 +02:00

kauffj (Migrated from github.com) left a comment

one small non-blocking objection

src/ui/component/sideBar/view.jsx

					
				@ -89,3 +89,3 @@

				              ...buildLink(PAGES.TRANSACTIONS, 'Transactions', ICONS.TRANSACTIONS),

				              ...buildLink(PAGES.TRANSACTIONS, __('Transactions'), ICONS.TRANSACTIONS),

				            },

				            {

kauffj (Migrated from github.com) commented

2019-05-14 19:24:48 +02:00

ty for this

src/ui/component/socialShare/view.jsx

kauffj (Migrated from github.com) commented

2019-05-14 19:26:50 +02:00

Heh, I think you changed this class because I complained, but I'd argue these aren't "help" items either.

card__label, card__action-label? or something like this? it's fine if the classes are styled exactly the same, but using help when it is not "help" will render future style evolution harder

Heh, I think you changed this class because I complained, but I'd argue these aren't "help" items either. `card__label`, `card__action-label`? or something like this? it's fine if the classes are styled exactly the same, but using `help` when it is not "help" will render future style evolution harder

Sign in to join this conversation.

No reviewers

kauffj