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.

Sign up for GitHub

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

Jump to bottom

Student: comment on MCQ answers: replace cancel button with a delete button #9030 #9033

Merged
merged 3 commits into from
Aug 10, 2018
Merged

Student: comment on MCQ answers: replace cancel button with a delete button #9030 #9033

merged 3 commits into from
Aug 10, 2018

Conversation

nidhi98gupta
Copy link
Contributor

Fixes #9030

@nidhi98gupta nidhi98gupta changed the title Student: Allow adding comments to MSQ answers #9031 Student: comment on MCQ answers: replace cancel button with a delete button #9030 Aug 9, 2018
@nidhi98gupta
Copy link
Contributor Author

nidhi98gupta commented Aug 9, 2018 •
edited
Loading

@xpdavid @LuLechuan Can do initial review before I make god mode changes?
uii

@xpdavid
Copy link
Contributor

xpdavid commented Aug 9, 2018

Should use cross icon rather than trash icon?

@damithc
Copy link
Contributor

damithc commented Aug 9, 2018

Should use cross icon rather than trash icon?

I'm OK with either.

@nidhi98gupta
Copy link
Contributor Author

@xpdavid Ready for review.

@xpdavid xpdavid self-assigned this Aug 10, 2018
xpdavid
xpdavid approved these changes Aug 10, 2018
View reviewed changes
Copy link
Contributor

@xpdavid xpdavid left a comment

Choose a reason for hiding this comment

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

LGTM

@xpdavid xpdavid added the s.FinalReview The PR is ready for final review label Aug 10, 2018
@xpdavid xpdavid requested a review from damithc August 10, 2018 02:34
@xpdavid xpdavid added the c.Task Other non-user-facing works, e.g. refactoring, adding tests label Aug 10, 2018
@damithc
Copy link
Contributor

damithc commented Aug 10, 2018

@nidhi98gupta can give a screenshot? Want to confirm if the letter x looks like an icon. I guess you tried both x and X?

@xpdavid
Copy link
Contributor

xpdavid commented Aug 10, 2018 •
edited
Loading

image
Currently, It looks like this.

@damithc
Copy link
Contributor

damithc commented Aug 10, 2018

A related question: when an existing comment is being edited, do we show both x (for cancel edit) and the trashcan (for deleting the comment entirely)?

@xpdavid
Copy link
Contributor

xpdavid commented Aug 10, 2018

I think only the x button.

image

@damithc
Copy link
Contributor

damithc commented Aug 10, 2018

I think only the x button.

OK I guess. User has to cancel the edit first and then delete it.

@xpdavid xpdavid added s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging and removed s.FinalReview The PR is ready for final review labels Aug 10, 2018
@xpdavid xpdavid merged commit 108130b into TEAMMATES:master Aug 10, 2018
@xpdavid xpdavid added this to the V6.9.0 milestone Aug 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xpdavid xpdavid xpdavid approved these changes

@damithc damithc damithc approved these changes

Assignees

@xpdavid xpdavid

Labels
c.Task Other non-user-facing works, e.g. refactoring, adding tests s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging
Projects
None yet
Milestone
V6.9.0
Development

Successfully merging this pull request may close these issues.
3 participants
@nidhi98gupta @xpdavid @damithc