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

fix attemp #45

Merged
merged 1 commit into from
Oct 2, 2024
Merged

Conversation

zhuoyuT
Copy link

@zhuoyuT zhuoyuT commented Sep 9, 2024

Fixes #0000

Pull Request Type

  • Bug fix (please link related issues if applicable)
  • New feature
  • Question update
  • Other

Summary

@danieldanielecki
Copy link
Member

@eduardconstantin, what do you think?

The user has reported a bug via Reddit, @zhuoyuT might you confirm that's what is the PR about?

Screenshot 2024-09-11 at 10 25 13

@eduardconstantin
Copy link

someone else implemented the scoring, I believe this happens because some questions have multiple answers and the score is divided by the number of answers

@zhuoyuT
Copy link
Author

zhuoyuT commented Sep 12, 2024

@danieldanielecki
Hey it is, but this is not final. I commented out the shuffle function and reduced the question count for debugging purpose, I will try to update it during the weekend, but feel free to adjust on top of my commit.

@eduardconstantin
the issue I encountered was the point of the last question was never added

@eduardconstantin
Copy link

even better

@danieldanielecki
Copy link
Member

@danieldanielecki Hey it is, but this is not final. I commented out the shuffle function and reduced the question count for debugging purpose, I will try to update it during the weekend, but feel free to adjust on top of my commit.

@eduardconstantin the issue I encountered was the point of the last question was never added

thanks for the clarification, will be waiting for the next commits :)

@zhuoyuT
Copy link
Author

zhuoyuT commented Sep 22, 2024

@eduardconstantin @danieldanielecki PR updated, please check

@danieldanielecki
Copy link
Member

@eduardconstantin @danieldanielecki PR updated, please check

Is it now 100% fixed & ready?

@danieldanielecki
Copy link
Member

What I understood before this fix when there was a question with multiple answers correct, the total result wasn't calculated correctly. Now it should be fixed, and easily noticed by seeing 100% for such tests in Practice Mode.

Might you please confirm?

cc: @eduardconstantin

@eduardconstantin
Copy link

What I understood before this fix when there was a question with multiple answers correct, the total result wasn't calculated correctly. Now it should be fixed, and easily noticed by seeing 100% for such tests in Practice Mode.

Might you please confirm?

cc: @eduardconstantin

I assumed that was the issue, I haven't checked

@danieldanielecki danieldanielecki merged commit a162e69 into Ditectrev:main Oct 2, 2024
@danieldanielecki
Copy link
Member

I tested, and it was indeed the case.

However, regardless of the number of multiple-choice questions, the maximum score was always 96.67/100. Merged.

@danieldanielecki
Copy link
Member

and it's no longer the case, thank you so much for the contribution @zhuoyuT

Screenshot 2024-10-03 at 10 23 02

@eduardconstantin, I also made the counter of answered questions in the top left a bit more explicit by adding ANSWERED: . Thank you as well, for the input!

@@ -19,7 +19,7 @@ const Modes: NextPage<{ searchParams: { url: string; name: string } }> = ({
<ExamLink
href={{
pathname: "/practice",
query: { url },
query: { url, name },
Copy link
Member

Choose a reason for hiding this comment

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

thanks for aligning it with the root page query

@eduardconstantin
Copy link

and it's no longer the case, thank you so much for the contribution @zhuoyuT

Screenshot 2024-10-03 at 10 23 02 @eduardconstantin, I also made the counter of answered questions in the top left a bit more explicit by adding `ANSWERED: `. Thank you as well, for the input!

not a bad idea, but now the top part is not centered anymore :(

@danieldanielecki
Copy link
Member

and it's no longer the case, thank you so much for the contribution @zhuoyuT
Screenshot 2024-10-03 at 10 23 02
@eduardconstantin, I also made the counter of answered questions in the top left a bit more explicit by adding ANSWERED: . Thank you as well, for the input!

not a bad idea, but now the top part is not centered anymore :(

uhh good feedback, missed it. Any idea how to handle both?

@danieldanielecki
Copy link
Member

and it's no longer the case, thank you so much for the contribution @zhuoyuT
Screenshot 2024-10-03 at 10 23 02
@eduardconstantin, I also made the counter of answered questions in the top left a bit more explicit by adding ANSWERED: . Thank you as well, for the input!

not a bad idea, but now the top part is not centered anymore :(

used emoji instead 5ac85c3 and now having best of both ;)

@danieldanielecki
Copy link
Member

What I understood before this fix when there was a question with multiple answers correct, the total result wasn't calculated correctly. Now it should be fixed, and easily noticed by seeing 100% for such tests in Practice Mode.

Might you please confirm?

cc: @eduardconstantin

I meant Exam Mode of course. Practice Mode has no results shown.

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.

4 participants