[DP-276] Comments Viewing FrontEnd #370
Conversation
✅ Deploy Preview for dailp ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| ? css.tagColorStory | ||
| : p.comment.commentType === Dailp.CommentType.Suggestion | ||
| ? css.tagColorSuggestion | ||
| : css.tagColorQuestion |
There was a problem hiding this comment.
One thing to consider: You can have named style variants that share some common styling but diverge based on a parameter. Using a variant might clean up things here and lead to less redundant css.
read more: https://vanilla-extract.style/documentation/api/style-variants/
There was a problem hiding this comment.
Hmm, I'm trying this and it's fine to define a variant but I'm having trouble actually passing in the values I need to use it from the component without getting errors. If this is essential I can keep trying to figure out how to do it but I've spent a while on it and am struggling
| ) : p.parent.__typename === "AnnotatedForm" ? ( | ||
| <WordCommentSection word={p.parent} /> | ||
| ) : ( | ||
| "" |
There was a problem hiding this comment.
parent cannot be undefined, null, or some other type. You can delete this embedded ternary to leave you with the following:
{p.parent.__typename === "DocumentParagraph" ? (
<ParagraphCommentSection paragraph={p.parent} />
) : <WordCommentSection word={p.parent} />)}
There was a problem hiding this comment.
when I try to delete the embedded ternary I get this error:
Type '{ readonly __typename?: "DocumentParagraph" | undefined; } & Pick<DocumentParagraph, "id" | "index" | "translation"> & { readonly source: readonly (({ readonly __typename: "AnnotatedForm"; } & Pick<...> & { ...; }) | { ...; })[]; readonly comments: readonly ({ ...; } & Pick<...>)[]; }' is not assignable to type 'FormFieldsFragment'.
Type '{ readonly __typename?: "DocumentParagraph" | undefined; } & Pick<DocumentParagraph, "id" | "index" | "translation"> & { readonly source: readonly (({ readonly __typename: "AnnotatedForm"; } & Pick<...> & { ...; }) | { ...; })[]; readonly comments: readonly ({ ...; } & Pick<...>)[]; }' is not assignable to type '{ readonly __typename?: "AnnotatedForm" | undefined; }'.
Types of property '__typename' are incompatible.
Type '"DocumentParagraph" | undefined' is not assignable to type '"AnnotatedForm" | undefined'.```
There was a problem hiding this comment.
It looks like typescript is not being nice to you. Sometimes nested fields will be like this when you try to do a type guard (checking if a value is of a certain type, like you do when you check the value of __typename) on a literal. You might see if this goes away if instead of taking in an object of params like this:
(p: {...prop type}): ReactElement
You instead "destructure" your props like this:
({parent}: {... prop type}): ReactElement
Here are some docs on "destructuring" in JS if that is new for you: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment
There was a problem hiding this comment.
We may also fix this if we upgrade typescript somehow. This should work.
|
@GracefulLemming I have addressed most of your comments and left follow-ups on two of them, lmk your thoughts! |
There was a problem hiding this comment.
Up to @GracefulLemming for final approval but this looks good to me.
| const commentTypeNames: Record<Dailp.CommentType, string> = { | ||
| // ... TS will then make sure you have an entry for everything on the "CommentTag" type that you import from the codegen | ||
| [Dailp.CommentType.Story]: "Story", | ||
| [Dailp.CommentType.Suggestion]: "Suggestion", | ||
| [Dailp.CommentType.Question]: "Question", | ||
| } |
There was a problem hiding this comment.
You could move this out of the function and make it a constant with SCREAM_CASE_NAMING if you wanted.
thanks for your comments Charlie! |
|
@hazelyn11 if you want to finish the suggestions Charlie made, feel free to. Otherwise, you can merge! |
Jira ticket
Now we can view comments both in the word pane and the paragraph pane!
One note-- in order to render comments immediately after they are submitted, I am currently also showing the collapsible Discussion panel even when it is empty. Otherwise, the pane has to be closed and re-opened in order for comments to show. Maybe there's a better way around this?