-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[DP-276] Comments Viewing FrontEnd #370
Changes from 5 commits
a502b2a
adac726
2726900
1180920
fd2d215
686a8c3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,81 @@ | ||
import { style, styleVariants } from "@vanilla-extract/css" | ||
import { margin } from "polished" | ||
import { | ||
colors, | ||
hspace, | ||
mediaQueries, | ||
radii, | ||
tagColors, | ||
thickness, | ||
vspace, | ||
} from "src/style/constants" | ||
import { marginX, marginY, paddingX, paddingY } from "src/style/utils" | ||
|
||
const wordShared = style([ | ||
paddingY(vspace.quarter), | ||
paddingX(hspace.halfEdge), | ||
{ | ||
borderWidth: thickness.thick, | ||
borderStyle: "solid", | ||
"@media": { | ||
[mediaQueries.print]: { | ||
...margin(0, "3.5rem", vspace[1.5], hspace.small), | ||
}, | ||
[mediaQueries.medium]: margin(0, "2.5rem", vspace.one, hspace.small), | ||
}, | ||
}, | ||
]) | ||
|
||
export const commentWrapper = style([ | ||
wordShared, | ||
marginY(vspace.half), | ||
{ | ||
borderColor: colors.borders, | ||
borderRadius: radii.large, | ||
lineHeight: vspace.one, | ||
pageBreakInside: "avoid", | ||
breakInside: "avoid", | ||
"@media": { | ||
[mediaQueries.medium]: {}, | ||
}, | ||
}, | ||
]) | ||
|
||
export const headerStyle = style([ | ||
marginX(hspace.medium), | ||
{ | ||
fontSize: "0.7rem", | ||
}, | ||
]) | ||
|
||
export const tagPadding = style([paddingY(vspace.medium)]) | ||
|
||
export const tagColorStory = style([ | ||
paddingX(hspace.medium), | ||
{ | ||
display: "inline-block", | ||
borderRadius: radii.round, | ||
backgroundColor: tagColors.story, | ||
fontSize: "0.7rem", | ||
}, | ||
]) | ||
|
||
export const tagColorSuggestion = style([ | ||
paddingX(hspace.medium), | ||
{ | ||
display: "inline-block", | ||
borderRadius: radii.round, | ||
backgroundColor: tagColors.suggestion, | ||
fontSize: "0.7rem", | ||
}, | ||
]) | ||
|
||
export const tagColorQuestion = style([ | ||
paddingX(hspace.medium), | ||
{ | ||
display: "inline-block", | ||
borderRadius: radii.round, | ||
backgroundColor: tagColors.question, | ||
fontSize: "0.7rem", | ||
}, | ||
]) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,101 @@ | ||
import React from "react" | ||
import * as Dailp from "src/graphql/dailp" | ||
import { TranslatedParagraph } from "src/segment" | ||
import * as css from "./comment-section.css" | ||
|
||
export const CommentSection = (p: { | ||
parent: Dailp.FormFieldsFragment | TranslatedParagraph | ||
}) => { | ||
return ( | ||
<div> | ||
{p.parent.__typename === "DocumentParagraph" ? ( | ||
<ParagraphCommentSection paragraph={p.parent} /> | ||
) : p.parent.__typename === "AnnotatedForm" ? ( | ||
<WordCommentSection word={p.parent} /> | ||
) : ( | ||
"" | ||
)} | ||
</div> | ||
) | ||
} | ||
|
||
export const WordCommentSection = (p: { word: Dailp.FormFieldsFragment }) => { | ||
const [{ data }] = Dailp.useWordCommentsQuery({ | ||
variables: { wordId: p.word.id }, | ||
}) | ||
|
||
const wordComments = data?.wordById.comments | ||
|
||
return ( | ||
<div> | ||
{wordComments?.map((comment) => ( | ||
<div> | ||
<CommentHeader comment={comment} /> | ||
<CommentBody comment={comment} /> | ||
</div> | ||
))} | ||
</div> | ||
) | ||
} | ||
|
||
export const ParagraphCommentSection = (p: { | ||
paragraph: TranslatedParagraph | ||
}) => { | ||
const [{ data }] = Dailp.useParagraphCommentsQuery({ | ||
variables: { paragraphId: p.paragraph.id }, | ||
}) | ||
|
||
const paragraphComments = data?.paragraphById.comments | ||
|
||
return ( | ||
<div> | ||
{paragraphComments?.map((comment) => ( | ||
<div> | ||
<div> | ||
<CommentHeader comment={comment} /> | ||
</div> | ||
<CommentBody comment={comment} /> | ||
</div> | ||
))} | ||
</div> | ||
) | ||
} | ||
|
||
export const CommentBody = (p: { comment: Dailp.Comment }) => { | ||
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", | ||
} | ||
Comment on lines
+65
to
+70
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You could move this out of the function and make it a constant with SCREAM_CASE_NAMING if you wanted. |
||
|
||
return ( | ||
<div className={css.commentWrapper}> | ||
{p.comment.textContent} | ||
<div className={css.tagPadding}> | ||
<div | ||
className={ | ||
p.comment.commentType === Dailp.CommentType.Story | ||
? css.tagColorStory | ||
: p.comment.commentType === Dailp.CommentType.Suggestion | ||
? css.tagColorSuggestion | ||
: css.tagColorQuestion | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.comment.commentType | ||
? commentTypeNames[p.comment.commentType as Dailp.CommentType] | ||
: ""} | ||
</div> | ||
</div> | ||
</div> | ||
) | ||
} | ||
|
||
export const CommentHeader = (p: { comment: Dailp.Comment }) => { | ||
return ( | ||
<div className={css.headerStyle}> | ||
{p.comment.postedBy.displayName} contributed on{" "} | ||
{p.comment.postedAt.date.formattedDate} | ||
</div> | ||
) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
parent
cannot beundefined
,null
, or some other type. You can delete this embedded ternary to leave you with the following:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when I try to delete the embedded ternary I get this error:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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:You instead "destructure" your props like this:
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may also fix this if we upgrade typescript somehow. This should work.