WordPress comments package

Some feedback for you, Michal! :grin:

  1. Store the validation errors returned from WordPress in the state .

    Yup, I also thought of storing that information in the state when I talked with Mario about using the REST API. My initial idea was to store the same error object received as it is in the submission status so you would access the validation errors doing something like this:

    form.status.error.data.params
    

    But if you think there is a benefit of creating a specific property in the state, that’s OK for me.

    I’m not sure about it is better for that property to be an array. How are those errors going to be consumed? Just printing an enumeration of problems for the whole form? Or an error message in each field?

  2. Add the newly submitted comment to state.source.comments directly .

    I agree with that. isOnHold and isApproved would not be needed anymore but at least we need a property to know if there was a successful request so we can print a message in React that the comment was published correctly. If not, there would not be any difference in the state if you have published comments or you have never done it.

    As I said before, I would store the same error object received from WordPress so I would change the status property to have the following structure:

    export interface Status {
      /**
      * Request has not finished yet.
      */
      isPending: boolean;
    
      /**
      * Request has finished successfully.
      */
      isSubmitted: boolean;
    
      /**
      * Request has failed.
      */
      isError: boolean;
    
      /**
      * The error object that was returned by the REST API.
      * (having WpError the same structure than the error response).
      */
      error: WpError;
    }
    

    Again, if you think it is better to not use the same structure WordPress is using for errors I have no problem with using our own schema.

    That would be really great. :blush:

  3. Provide an updateField() action .

    Actually, the current implementation of updateFields() allows you to update a single field, you just need to pass an object with that property, e.g.

    updateFields({ email: "my.other@mail.com" });
    

    If you think it is confusing we can add the updateField() action, no problem with that. :+1:


BTW, I really like your proposal for future API improvement. :clap:

I like the idea of useComments for the future :slightly_smiling_face:

I’ve renamed it to useComments instead of useWPComments because that hook would need to by provided by any comments package.


One question: Can we add the unapproved comments to state.source.comments as well? Are they returned properly from the the REST API?

In that case, I think we wouldn’t need an array of submissions because the unapproved comments would be stored in state.source.comments, is that right?

Another question: Maybe due to that we don’t need a submitted or status field anymore and we can have those props in Form:

export interface Form {
  fields: Fields;
  isSubmitting: boolean;
  isSubmitted: boolean;
  isError: boolean;
  errorMessage?: string;
  errorCode?: string;
  errorStatusCode?: number;
  errors: Errors;
}

The unapproved comments are returned by the REST API with the same format as accepted comments. The only difference is the value in the status field, that would be hold in that case.

Awesome. Then I guess we don’t need an array of submissions anymore :slight_smile:


By the way, what do you think about renaming isPending to isSubmitting because the action would be actions.comments.submit and I’d like to keep consistency with isFetching that relates to actions.source.fetch.

Yeah, I would also try to keep the errors schema in the state as close as possible to what is returned by WordPress.

Yeah, I think I will change it to an object. This way, yes you have to do a Object.entries() when enumerating errors, but I think normally you will want show an error individually for each field an it’s easier with an object.

:+1:

The unapproved comments are only returned by the REST API if you are authenticated though… If you are not authenticated, you cannot change the default status=approve :

Would this be a problem? I think it’s fine:

If the user is not authenticated then once they submit a comment we can add it to state.source.comments. This way, we can show a comment that is on hold at least temporarily. Then:

  • The comment can get either approved or thrashed in the meantime but the user has to refresh the page to see the “updated” state.
  • If the comment is still on hold, the user will not see it unless we store it in localstorage, but again I think that it’s fine.

That sounds fine to me, I agree that a more flat schema is better.

2 Likes

This has finally been implemented in https://github.com/frontity/frontity/pull/542

This is the recap of the changes:

  • Update the wp-comments package to use the REST API directly
  • Add the data returned by the REST API when POSTing a new comment directly to state.source.data and state.source.comment . This is handled inside of the actions.comments.submit() action.
  • Store the validation errors returned from WordPress in the state. The REST API returns the validation errors for each of the fields in the comment. E.g. if you send a request with a malformed author_email
  • Remove the query , route & link from the non-URL entities (like the comments) that are fetched from the REST API. The link for those entities starts with a @, e.g. @comments/42

You can now use the package to fetch and create new comments.

A pretty full usage example (please note that it’s not tested, so it might contain typos):

const Comments = ({ actions, state, postId }) => {
  useEffect(() => {
    actions.source.fetch(`@comments/${postId}`);
  }, []);

  const data = state.source.get(`@comments/${postId}`);
  const form = state.comments.forms[postId];

  return data.isReady ? (
    <>
      <form
        onSubmit={(e) => {
          e.preventDefault();

          // Post the comment using that values present in
          // state.comments.forms[postId].fields
          //
          // You can also pass the values as a parameter to `submit()`, like:
          // submit(1, {content: '', authorName: "Mike", authorEmail: hi@test.com })
          actions.comments.submit(postId);
        }}
      >
        {/* If the form is submitting, we can show some kind of a loading state */}
        {form.isSubmitting && <Loading />}

        <input
          name="content"
          onChange={(e) =>
            actions.comments.updateFields(postId, { content: e.target.value })
          }
          value={state.comments.forms[postId]?.fields?.content || ""}
        />

        {/* Show the errors for the individual fields.
            E.g. content of a comment cannot be empty and the email must be valid */}
        {form.errors?.content}

        <input
          name="author_name"
          onChange={(e) =>
            actions.comments.updateFields(postId, {
              authorName: e.target.value,
            })
          }
          value={state.comments.forms[postId]?.fields?.authorName || ""}
        />
        {form.errors?.authorName}

        <input
          name="author_email"
          onChange={(e) =>
            actions.comments.updateFields(postId, {
              authorEmail: e.target.value,
            })
          }
          value={state.comments.forms[postId]?.fields?.authorEmail || ""}
        />
        {form.errors?.authorEmail}

        {/* Show the REST API error messages.
            E.g. "Sorry, you must be logged in to comment." */}
        <div>ERROR: {form.errorMessage}</div>

        {/* If the form was submitted successfully we can show a confirmation */}
        <div>{form.isSubmitted && "The comment submitted successfully!"}</div>

        <input type="submit" />
      </form>

      {/* Display the list of comments */}
      <div>
        {data.items.map(({ id, children }) => {
          if (children) {
            // Iterate over the sub-comments jand render them...
          }
          return (
            <>
              <div> {state.source.comment[id].author_name} said: </div>
              <div> {state.source.comment[id].content} </div>
            </>
          );
        })}
      </div>
    </>
  ) : null;
};
1 Like

Hey @mmczaplinksi, amazing work :slightly_smiling_face:

I have a comment about this, though. I think we are assuming how non-URL data objects are going to be used, and restricting their usage too early.

As a general rule, I’d prefer trying not to restrict the APIs as much as we can.

In this specific case, I also see possible uses for the removed fields.

  • link: The goal of introducing this field was to make it easier to use it as a self-reference, useful when passing objects around.

    For example, imagine a function that receives the data object.

    const sendSomeEvent = (data) => {
      someAnalyticsLibrary.send({ name: data.link, payload: data });
    };
    
    sendSomeEvent(data);
    

    I think that usage still applies for non-URL data objects.

  • route: This is required for pagination, the same way page is required.

    Imagine we introduce pagination in the comments, for example.

    await actions.source.fetch("@comments/10/page/2");
    const data = state.souce.get("@comments/10/page/2");
    

    It would make sense to have both data.page (2) and data.route ("@comments/10") in the data object, just like in any other paginated data object.

  • query: This gives an opportunity to add options.

    We decided to use a string for the index of the data object. I think that’s a great choice because they are much simpler to query and identify than JavaScript objects. But these string indexes need to be very versatile.

    Thankfully, the URL-format is, in a sense, similar to a JavaScript object. For optional options, the query is ideal, and people are quite used to on the web, so it’s easy to understand.

    Imagine we want to introduce some options to the @comments requests.

    • Possibility to filter by the author of the comments.
    • Possibility to filter the nested level of the comments fetched.

    If we don’t have the query, we need to bake those in the path:

    pattern: `@comments/:postId/:author/:nestedLevel`
    

    But if those options are optional, maybe I don’t want to filter by an author, I only want to specify a nested level. I have no way to do so. It will also become quite convoluted when there are many options.

    With queries, it becomes much simpler. The pattern is back to:

    pattern: `@comments/:postId`
    

    And queries are handled inside the handler. If I want to use the nested level option but not the author one, I could do:

    actions.source.fetch("@comments/60?nestedLevel=2")
    

Sorry for the long explanation :sweat_smile: I was trying to make my point clear.

Those are all really good points, Luis, thank you! :slight_smile:

I’m happy to undo that change now or we could wait for the next opportunity when we are doing further work on wp-comments.

@SantosGuillamot what do you think?

If it’s a small change I would do it now. I haven’t released this version yet, so if you think it’s something we have to do, I’d include it before announcing it. I was planning to release and announce it once the new Yoast package is merged, but it can wait until tomorrow if this change doesn’t take long.

1 Like

I ll PR to undo this so that you can merge it before announcing the new release.

1 Like

Thanks for the PR Michal.

It’s not that this is that important, because the @comments/:post-id pattern introduced doesn’t use pagination or queries, but I think it’ll be important for future non-URL patterns made by us or the community :slightly_smiling_face:

This feature, the beta version, has been released :tada: : Read the full Release Announcement.

Following @mmczaplinski 's code, I created a CodeSandbox to show an example implementing it. I recommend you to point it to your own WordPress site, so you can test the different settings and submit new comments (it isn’t currently allowed in test.frontity.org).

We also created a demo explaining a bit this example and how the package works:

1 Like

I love the video @santosguillamot! It’s really well explained :clap:

The only suggestion I have is to add a check before adding the comments to your theme so it works without a comments package.

Maybe check state.comments before adding the main component:

state.comments && <Comments postId={post.id} />;

And maybe check actions.comments?.submit before adding the form:

actions.comments?.submit && <CommentsForm postId={postId} />;

Two other things:

  • Should we open a FD for the comments namespace to start discussing it?
  • Should we open FDs for other wp-comments features, like the Comments component, a setting to fetch comments in SSR and so on?

Hello Frontity team,

Thank you so much for releasing this package which supports WordPress comments. :clap:t2:
I’ve implemented it and all worked fine. Please check it out here:

However, one thing i’ve noticed is that, when i navigate through the other posts, the comments section is not rendered.

Scenario:

  1. Go to the Link i provided above
  2. Scroll down to “READ NEXT” section which is right after the comments section
  3. Click on some of the posts e.g.“How To Make Easy Homemade Pizza Dough”

You’ll find out that the comments section is not appearing, but after reloading the page.

Any advices on this?

Thanks so much.
Best,
Dejan

@dejangeorgiev is the repo available to check if this is a bug in the comments package or the error is somewhere else?

By the way, they look awesome :slightly_smiling_face:

Hi @luisherranz :wave:t3:

Thank you very much! :nerd_face:
Yes, here is the repo :point_right:t2: https://github.com/dejangeorgiev/ruthgeorgiev-frontity

Thanks @dejangeorgiev.

@mmczaplinski could you please check it out?

Hi @dejangeorgiev

It’s a bit hard to debug this as I’m looking at a few issues that are obscuring what is going on for me :sweat_smile:

  1. The comments form shows up correctly for me e.g. when I go to localhost:3000/how-to-make-perfect-carrot-cupcakes/ directly.

  2. However, the “Recommendations” section does not actually show up at all!

  3. When I open up the console, I get the following error:

ServerError: post type from endpoints "posts,pages,media" with slug "388" not found
    at Object.eval (webpack-internal:///./node_modules/@frontity/wp-source/src/libraries/handlers/postType.ts:37:21)
    at Generator.next (<anonymous>)
    at asyncGeneratorStep (webpack-internal:///./node_modules/@frontity/wp-source/src/libraries/handlers/postType.ts:4:1069)
    at _next (webpack-internal:///./node_modules/@frontity/wp-source/src/libraries/handlers/postType.ts:4:1383)
    at eval (webpack-internal:///./node_modules/@frontity/connect/src/scheduler.js:7:128)
    at batchedUpdates$1 (webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:3683:146)
    at batch (webpack-internal:///./node_modules/@frontity/connect/src/scheduler.js:7:113)
    at batched (webpack-internal:///./node_modules/@frontity/connect/src/scheduler.js:10:261)

I think that you want to load the data from the recipes endpoint and not the posts, pages or media. Looks like you might have a misconfigured handler here, I’m not quite sure :slight_smile:

  1. I’ve noticed that when I browse with adblock on (uBlock origin), the page loads and then goes blank because the adsbygoogle object is not present in the window. This might actually be an error on our side, but we’ll have to investigate!

Are you able to reproduce the issues that I mention ?

1 Like