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?
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.
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.
BTW, I really like your proposal for future API improvement.
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:
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
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.
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 :
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.
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;
};
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:
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.
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
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:
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
The comments form shows up correctly for me e.g. when I go to localhost:3000/how-to-make-perfect-carrot-cupcakes/ directly.
However, the “Recommendations” section does not actually show up at all!
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
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 ?