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

[#12508] Give respondents a way to reset a rubric question submission #12569

Closed
wants to merge 11 commits into from
Closed

[#12508] Give respondents a way to reset a rubric question submission #12569

wants to merge 11 commits into from

Conversation

Mex7180
Copy link
Contributor

@Mex7180 Mex7180 commented Aug 30, 2023

Fixes #12508

Outline of Solution

Added a new reset button connected to a function resetRubricQuestion() changing the currently shown answers to the empty question state (not automatically saved in backend).
Changed the media queries, because if the window size was changed more than once, at the with of 768px, the rubric questions were not shown anymore.

Different behavior shown in the video (resetting the question without saving, resetting the question with saving)
https://github.com/TEAMMATES/teammates/assets/90850610/a07e7b65-d2ae-472a-8f9a-10b00c202a18

@Mex7180
Create reset button for rubric question type
42275ce 
- reset button added
- function resetRubricQuestion() added to rubric-question-edit-answer-form module
- styling for the button
@Mex7180 Mex7180 changed the title [#12508] Update: Give respondents a way to reset a rubric question submission [#12508] Give respondents a way to reset a rubric question submission Aug 30, 2023
@jasonqiu212 jasonqiu212 added the s.ToReview The PR is waiting for review(s) label Sep 1, 2023
@Mex7180
Copy link
Contributor Author

Mex7180 commented Sep 4, 2023

Ready for review :D

cedricongjh
cedricongjh reviewed Sep 4, 2023
View reviewed changes
Copy link
Contributor

@cedricongjh cedricongjh left a comment

Choose a reason for hiding this comment

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

Hey @Mex7180, thank you for your PR! Just a couple comments:

  1. As stated in the issue, there should add a confirmation modal for the user to confirm a reset.
  2. It would be good if there's a test added for the resetRubricQuestion function

Do make the changes and re-request for a review, thank you!

@Mex7180
Copy link
Contributor Author

Mex7180 commented Sep 4, 2023

Hi @cedricongjh
I'll check on that. Do you know of any other cases where a confirmation modal was implemented? Just to keep consistent with the codebase.
Thanks for the feedback.

@cedricongjh
Copy link
Contributor

Hi @cedricongjh I'll check on that. Do you know of any other cases where a confirmation modal was implemented? Just to keep consistent with the codebase. Thanks for the feedback.

Do take a look at SimpleModalService and it's usages :D

@jasonqiu212 jasonqiu212 added s.Ongoing The PR is being worked on by the author(s) and removed s.ToReview The PR is waiting for review(s) labels Sep 9, 2023
@Mex7180
Copy link
Contributor Author

Mex7180 commented Sep 12, 2023

Update on the current state:
I've created the pop-up/modal as a new module and could also import it at the right place. I'm now trying to connect the reset function to the modal, but I'm too busy this week, so I'll only be able to work on this next week.

@Mex7180
Copy link
Contributor Author

Mex7180 commented Sep 17, 2023

Update on the current state:
I've decided to use an other approach with a decorator to implement a more widely-usable confirm modal/dialog. I've decided to try this because I kept on failling with connecting the buttons of the dialog with the rubric-question-component.
I hope this will fullfill the requirements and be more usable.

@cedricongjh
Copy link
Contributor

hi @Mex7180, the code on the PR does not contain the confirmation modal/dialog, wondering if you have pushed it yet?

@Mex7180
Copy link
Contributor Author

Mex7180 commented Sep 25, 2023 •
edited
Loading

Hi @cedricongjh
Sorry for the confusion, I just updated the branch to get the newest changes from the master branch. I'm still working locally on it.
Gonna keep you updated on the progress.

@nusoss-bot
Copy link

Folks, This PR seems to be stalling (no activities for the past 7 days). 🐌 😢
Hope someone can get it to move forward again soon...

1 similar comment
@nusoss-bot
Copy link

Folks, This PR seems to be stalling (no activities for the past 7 days). 🐌 😢
Hope someone can get it to move forward again soon...

@Mex7180 Mex7180 closed this Oct 12, 2023
@Mex7180
Copy link
Contributor Author

Mex7180 commented Oct 12, 2023 •
edited
Loading

Closing this PR because I don't find the time to work on it, I hope someone can get it to move forward.

@NeoHW NeoHW mentioned this pull request Jul 14, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cedricongjh cedricongjh cedricongjh left review comments

Assignees
No one assigned
Labels
s.Ongoing The PR is being worked on by the author(s)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Give respondents a way to reset a rubric question submission
4 participants
@Mex7180 @cedricongjh @nusoss-bot @jasonqiu212