Thumbnail edit improvements #1586

Merged
neb-b merged 4 commits from edit-thumbnail into master 2018-06-14 23:08:03 +02:00
neb-b commented 2018-06-13 06:11:43 +02:00 (Migrated from github.com)
https://github.com/lbryio/lbry-app/issues/1584
tzarebczan commented 2018-06-14 18:48:23 +02:00 (Migrated from github.com)

This looks good functionally according to my testing!

This looks good functionally according to my testing!
kauffj (Migrated from github.com) requested changes 2018-06-14 21:42:51 +02:00
kauffj (Migrated from github.com) left a comment

Aside from improving the name of STATUSES, I would not consider any of this feedback to be necessary to address to merge.

Aside from improving the name of `STATUSES`, I would not consider any of this feedback to be necessary to address to merge.
@ -2,3 +2,3 @@
import * as React from 'react';
import { isNameValid, buildURI, regexInvalidURI } from 'lbry-redux';
import { isNameValid, buildURI, regexInvalidURI, STATUSES } from 'lbry-redux';
import { Form, FormField, FormRow, FormFieldPrice, Submit } from 'component/common/form';
kauffj (Migrated from github.com) commented 2018-06-14 21:37:46 +02:00

what kind of statuses are these?

what kind of statuses are these?
kauffj (Migrated from github.com) commented 2018-06-14 21:41:31 +02:00

How feasible (and beneficial) would it be to split the entirety of this into a separate component?

How feasible (and beneficial) would it be to split the entirety of this into a separate component?
kauffj (Migrated from github.com) commented 2018-06-14 21:40:51 +02:00

This design forces the user to wait for the upload to finish before clicking Publish and it may not be clear why they're waiting.

It would be preferable if Publish could be clicked regardless of the upload status and instead the app waited for the remote upload to finish before making the publish API call.

(Fixing this may be tricky, and if it's complicated probably shouldn't be considered blocking.)

This design forces the user to wait for the upload to finish before clicking Publish and it may not be clear why they're waiting. It would be preferable if Publish could be clicked regardless of the upload status and instead the app waited for the remote upload to finish before making the `publish` API call. (Fixing this may be tricky, and if it's complicated probably shouldn't be considered blocking.)
neb-b (Migrated from github.com) reviewed 2018-06-14 22:12:11 +02:00
neb-b (Migrated from github.com) commented 2018-06-14 22:12:11 +02:00

Not hard. We could just move this into the SelectThumbnail component.

Not hard. We could just move this into the `SelectThumbnail` component.
neb-b (Migrated from github.com) reviewed 2018-06-14 22:12:34 +02:00
@ -2,3 +2,3 @@
import * as React from 'react';
import { isNameValid, buildURI, regexInvalidURI } from 'lbry-redux';
import { isNameValid, buildURI, regexInvalidURI, STATUSES } from 'lbry-redux';
import { Form, FormField, FormRow, FormFieldPrice, Submit } from 'component/common/form';
neb-b (Migrated from github.com) commented 2018-06-14 22:12:34 +02:00

spee.ch api status'. Agreed we should probably rename.

spee.ch api status'. Agreed we should probably rename.
neb-b (Migrated from github.com) reviewed 2018-06-14 22:13:13 +02:00
neb-b (Migrated from github.com) commented 2018-06-14 22:13:13 +02:00

Until we move add this functionality somewhere else I think it's ok right now

Until we move add this functionality somewhere else I think it's ok right now
kauffj (Migrated from github.com) reviewed 2018-06-14 22:23:49 +02:00
kauffj (Migrated from github.com) commented 2018-06-14 22:23:49 +02:00

👍

:+1:
Sign in to join this conversation.
No reviewers
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#1586
No description provided.