-
Notifications
You must be signed in to change notification settings - Fork 480
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
Javalab: code review UI #41478
Javalab: code review UI #41478
Conversation
@@ -1,7 +1,16 @@ | |||
import React, {Component} from 'react'; | |||
//import PropTypes from 'prop-types'; |
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.
nit: remove this?
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's going to be re-added when I start including real comments as props, which is imminently 🤞
commentContainer: { | ||
margin: '0 0 25px 0' | ||
}, | ||
projectOwnerComment: { |
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.
I think we want the background to be blue when the comment is from 'you' (so if you are a peer/teacher and are looking at your comment on someone else's project, or if it's your own project and your own comment). Is that right @moneppo?
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.
Ooh that would make sense!
const teacherCommentSuffix = ` (${javalabMsg.onlyVisibleToYou()})`; | ||
return ( | ||
<span> | ||
<span style={styles.name}>{name}</span> |
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.
per the figma I think we want this to be 'you' if it is your comment
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.
LGTM!
Basic UI components for Javalab code review feature -- a
Comment
component to render a single comment, and aCommentEditor
component to create a new comment and submit it.Testing story
Added unit tests for behavior added in this PR (eg, different background colors on comments based on who the comment is from). More tests to come as this gets hooked up to the back end.
Follow-up work
The UI isn't quite perfectly lined up with the Figma mocks, so probably some additional work to get things to look juust right (eg, date text overflows when a comment is from a teacher). I also did not include the menu that will be shown when a user clicks on the ellipsis yet.
I'll also delete the demo comments as I start working on getting this to work with the back end.