[redesign] Publish improvements #1040

neb-b · 2018-02-23T08:26:55Z

LET THERE BE REDUX

Changes

Moved publish form values to redux
Move pending publish logic to redux

Issues

There are a few minor bugs but I would rather file those as existing issues and keep moving forward.

I am having issues with publishing under a channel but Tom can successfully do it (just my machine? I'm seeing some certificate error)
A few times Tom and I have seen "insufficient funds" errors from the daemon even if you have enough funds (possibly an issue with the number input or the value being coerced to a string)

Additional comments

Not super happy with the types. It feels like a lot of duplication. Additionally I'm just keeping the types in the reducer file, until Akinwale's PR to see how he goes about that.

neb-b · 2018-02-27T06:04:37Z

src/renderer/page/publish/index.js

+
+  let prefillClaim;
+  if (id) {
+    prefillClaim = selectClaimById(id)(state);


I think I'm probably going to change this and have whoever links to the publish page be responsible for populating the form before navigating. Currently there can be issues with this if you navigate away from the form and come back to an edit.

possibly

// file page populatePublishForm(fileInfo); doNavigate('/publish')

kauffj

As much feedback as I could give before app meeting deadline... still didn't actually run the code and didn't quite finish.

kauffj · 2018-02-27T15:08:17Z

src/renderer/component/address/view.jsx

+          />
+          <Button
+            alt
+            icon="Clipboard"


Use constants/icons or decide we don't deem this necessary

kauffj · 2018-02-27T15:10:38Z

src/renderer/component/common/form-components/form-field.jsx

 };

 export class FormField extends React.PureComponent<Props> {
  render() {
    const { render, label, prefix, postfix, error, helper, name, type, children, stretch, ...inputProps } = this.props;

-    // Allow a type prop to determine the input or more customizability with a render prop
    let Input;


Why capital? To represent a JSX element?

It was capital when I had the render function. Now that I removed it I'll set it back to lowercase.

kauffj · 2018-02-27T15:14:28Z

src/renderer/component/fileCard/index.js

+  let isResolvingUri;
+
+  // If a publish is pending, don't bother with this because props.uri isn't the complete uri at that moment
+  if (!props.pending) {


Why pass pending in rather than select it from Redux?

kauffj · 2018-02-27T15:17:58Z

src/renderer/component/fileList/view.jsx

  value?: {
    publisherSignature: {
      certificateId: string,
    }
  },
-  metadata: {
+  metadata?: {


Is this metadata? because the API is inconsistent about returning metadata? In general, I'm in favor of consistent signatures that include null or otherwise empty values (e.g. []), rather than omitting fields entirely.

That part specifically is for pending publishes. A pending publish won't have the metadata object. Maybe the issue is not keeping it the same. I know there was a lot of code to build an actual dummy claim in lbry.js, currently pendingPublishes[0] is the payload we send to lbry.publish

Yeah, I'm going to add the dummy claim stuff back inside the pending publish. I ran into a couple issues with it, and I'm sure I will only run into more

kauffj · 2018-02-27T15:19:07Z

src/renderer/component/fileList/view.jsx

@@ -70,8 +71,9 @@ class FileList extends React.PureComponent<Props, State> {
  getChannelSignature = (fileInfo: FileInfo) => {
    if (fileInfo.value) {
      return fileInfo.value.publisherSignature.certificateId;
+    } else if (fileInfo.metadata) {


What's going on here that this can have two different data structures?

If the daemon is inconsistent with it's responses, I'd like to get issues opened to clean this up.

kauffj · 2018-02-27T15:54:27Z

src/renderer/component/selectChannel/view.jsx

+    const channel = event.target.value;
+
+    if (channel === 'new') {
+      this.setState({ addingChannel: true });


Now that the publish page generally retains state via Redux, will this internal component state be a potential source of error or confusion?

This might be a good component to test out your idea, just to see how it works in practice. I'll keep it as is for now, but look into refactoring when we add it to the "Your publishes" page, or somewhere else.

kauffj · 2018-02-27T15:54:51Z

src/renderer/component/selectChannel/view.jsx

+                <option key="anonymous" value="anonymous">
+                  {__('Anonymous')}
+                </option>
+                {channels.map(({ name }) => (


It'd be cool if this defaulted to the last channel you published with.

That's a good idea

kauffj · 2018-02-27T15:55:54Z

src/renderer/lbry.js

- * needed to make a dummy claim or file info object.
- */
-let pendingId = 0;
-function savePendingPublish({ name, channelName }) {


kauffj · 2018-02-27T15:57:03Z

src/renderer/page/publish/index.js

+
+  let prefillClaim;
+  if (id) {
+    prefillClaim = selectClaimById(id)(state);


kauffj · 2018-02-27T15:57:54Z

src/renderer/lbry.js

-      errorCallback(err);
-    }
-  );
+Lbry.publishContent = (params, publishedCallback, errorCallback) => {


I think you can drop this and just use lbry.publish(...).then(...) in code.

neb-b · 2018-02-28T17:19:05Z

@kauffj Realized I didn't add contstants for 'anonymous' and 'new' for channel creation. And also forgot to fix the price placeholder after your comments. I'll add those.

add wallet balance background image

7273497

neb-b self-assigned this Feb 23, 2018

neb-b force-pushed the redesign-wip branch 4 times, most recently from 0e6c6d9 to d5dc21d Compare February 27, 2018 05:31

neb-b commented Feb 27, 2018

View reviewed changes

lbry-bot unassigned neb-b Feb 27, 2018

neb-b force-pushed the redesign-wip branch from d5dc21d to ceb67c7 Compare February 27, 2018 06:26

move publish logic to redux and implement redesign

c982779

neb-b force-pushed the redesign-wip branch from ceb67c7 to c982779 Compare February 27, 2018 08:27

neb-b requested a review from kauffj February 27, 2018 08:36

lbry-bot assigned kauffj Feb 27, 2018

neb-b requested a review from liamcardenas February 27, 2018 08:36

lbry-bot assigned liamcardenas Feb 27, 2018

kauffj requested changes Feb 27, 2018

View reviewed changes

lbry-bot unassigned kauffj Feb 27, 2018

create publish modal

41afa7c

neb-b force-pushed the redesign-wip branch from 902db96 to 41afa7c Compare February 27, 2018 23:08

use constants for licences and bid amount

c5fc691

neb-b requested a review from kauffj February 28, 2018 01:22

lbry-bot assigned kauffj and unassigned kauffj Feb 28, 2018

use constants for channel names and lower initial bid amount/placeholder

79665f1

neb-b force-pushed the redesign-wip branch from 026ae40 to 79665f1 Compare February 28, 2018 19:36

neb-b merged commit 74ea22c into redesign Mar 1, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[redesign] Publish improvements #1040

[redesign] Publish improvements #1040

neb-b commented Feb 23, 2018 •

edited

Loading

neb-b Feb 27, 2018

kauffj Feb 27, 2018

kauffj left a comment

kauffj Feb 27, 2018

kauffj Feb 27, 2018

neb-b Feb 27, 2018

kauffj Feb 27, 2018

neb-b Feb 27, 2018

kauffj Feb 27, 2018

neb-b Feb 27, 2018

neb-b Feb 27, 2018

kauffj Feb 27, 2018

kauffj Feb 27, 2018

neb-b Feb 28, 2018

kauffj Feb 28, 2018

kauffj Feb 27, 2018

neb-b Feb 27, 2018

kauffj Feb 27, 2018

kauffj Feb 27, 2018

kauffj Feb 27, 2018

neb-b commented Feb 28, 2018

[redesign] Publish improvements #1040

[redesign] Publish improvements #1040

Conversation

neb-b commented Feb 23, 2018 • edited Loading

LET THERE BE REDUX

Changes

Issues

Additional comments

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kauffj left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neb-b commented Feb 28, 2018

neb-b commented Feb 23, 2018 •

edited

Loading