Skip to content
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

Add code review UI messages #41781

Merged
merged 3 commits into from
Jul 30, 2021
Merged

Add code review UI messages #41781

merged 3 commits into from
Jul 30, 2021

Conversation

sanchitmalhotra126
Copy link
Contributor

Adds UI messages to the code review window and some associated view/style reorganization. Messages are shown based on whether the student has comments in the project and if peer review is enabled or not. Note - this only includes messages for a student viewing their own project. For students viewing another student's project, there is a different header message that is still TODO (pending peer review dropdown).

Peer.Review.Messages.UI.7.28.mov

Links

https://codedotorg.atlassian.net/browse/CSA-557

Testing story

Tested locally.

Deployment strategy

Follow-up work

Privacy

Security

Caching

PR Checklist:

  • Tests provide adequate coverage
  • Privacy and Security impacts have been assessed
  • Code is well-commented
  • New features are translatable or updates will not break translations
  • Relevant documentation has been added or updated
  • User impact is well-understood and desirable
  • Pull Request is labeled appropriately
  • Follow-up work items (including potential tech debt) are tracked and linked

@sanchitmalhotra126 sanchitmalhotra126 requested review from bencodeorg and a team July 28, 2021 18:49
@sanchitmalhotra126 sanchitmalhotra126 requested a review from a team as a code owner July 28, 2021 18:49
<div style={styles.commentsSection}>
<div style={styles.messageText}>{javalabMsg.feedbackBeginning()}</div>
{this.renderComments(comments, !isReadyForReview)}
{this.renderCommentEditor(forceRecreateEditorKey)}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice reorganization!

marginBottom: '25px',
textAlign: 'center',
fontStyle: 'italic',
color: '#818181'
Copy link
Contributor

@jmkulwik jmkulwik Jul 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where possible, prefer one of our existing colors, listed here: https://github.com/code-dot-org/code-dot-org/blob/staging/shared/css/color.scss

Copy link
Contributor

@jmkulwik jmkulwik Jul 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They can be used like this:

import color from '@cdo/apps/util/color';

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah nice, thanks! These are helpful - I didn't find the exact same grey in this file, but I can use something similar if it's not noticeably different or add this to colors.scss

Copy link
Contributor

@jmkulwik jmkulwik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! Leaving it up to you if integrating the color update makes sense.

@sanchitmalhotra126 sanchitmalhotra126 merged commit e60180c into staging Jul 30, 2021
@sanchitmalhotra126 sanchitmalhotra126 deleted the code-review-messages branch July 30, 2021 01:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants