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 question listing and navigation in quiz reports #12359

Merged
merged 4 commits into from
Jun 28, 2024

Conversation

rtibbles
Copy link
Member

@rtibbles rtibbles commented Jun 25, 2024

Summary

  • Annotate sections with start and end section numbers.
  • Simplify attempt log listing by requiring sections, so only having a mobile and non-mobile condition
  • Fix attempt log listing by including the section context in the question numbers.
  • Remove background to show selection state.
  • Update question number display to show the number globally for the quiz, rather than per section.
  • Update learner quiz display to be correlated.
  • Cleanup cruft in learner quiz display that was making our computed props over complicated and redundant.

References

Fixes #12334

Reviewer guidance

  • Test multi-section quiz reports to ensure that selection state is properly shown.
  • Test to ensure that navigation between items occurs properly.
  • Regression test practice quiz reports for learners and coaches
  • Regression test exercise reports for coaches
  • Ensure that multi-section quiz engagement and question selection functions properly.

Testing checklist

  • Contributor has fully tested the PR manually
  • If there are any front-end changes, before/after screenshots are included
  • Critical user journeys are covered by Gherkin stories
  • Critical and brittle code paths are covered by unit tests

PR process

  • PR has the correct target branch and milestone
  • PR has 'needs review' or 'work-in-progress' label
  • If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
  • If this is an important user-facing change, PR or related issue has a 'changelog' label
  • If this includes an internal dependency change, a link to the diff is provided

Reviewer checklist

  • Automated test coverage is satisfactory
  • PR is fully functional
  • PR has been tested for accessibility regressions
  • External dependency files were updated if necessary (yarn and pip)
  • Documentation is updated
  • Contributor is in AUTHORS.md

Simplify and fix attempt log listing.
Remove background to show selection state.
@pcenov
Copy link
Member

pcenov commented Jun 26, 2024

Hi @rtibbles, everything seems ok but noticed 2 issues that can potentially cause confusion:

  1. When all the sections are collapsed there's no indication to which section the displayed question belongs so that the learner/coach might have to expand several sections before finding out. Perhaps there should be a selection effect to the section title as well:

2024-06-26_16-22-59

  1. There's a mismatch between the question title within the section panel and the question title as displayed when viewing the details of the question. For example when I select Question 4 from the second section, it's displayed as Question 1 to the right and that's so for every new section:
2024-06-26_16-29-09.mp4

@rtibbles
Copy link
Member Author

Thanks @pcenov - definitely useful to add an indicator at the section level too - I will use the same background colour.

Will make sure that the titles match!

Have global question numbers in reports.
Add section descriptions if they exist.
…h reports.

Cleanup unnecessary props, computed props, and vuex getters.
@github-actions github-actions bot added the APP: Learn Re: Learn App (content, quizzes, lessons, etc.) label Jun 26, 2024
@rtibbles
Copy link
Member Author

Hi @pcenov I have fixed both these issues, and moved to a consistent 'global across the quiz' question numbering.

To be completely consistent, I have also updated this for learners taking the quiz too, so additional testing there would be very useful.

Copy link
Member

@pcenov pcenov left a comment

Choose a reason for hiding this comment

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

Thanks @rtibbles LGTM!

Copy link
Member

@marcellamaki marcellamaki left a comment

Choose a reason for hiding this comment

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

Holding the context for all of this is a bit of a challenge, but that's a factor of the complexity of some of this code. One non-blocking suggestion for an additional comment

kolibri/core/assets/src/exams/utils.js Show resolved Hide resolved
@rtibbles rtibbles merged commit 0a5ece1 into learningequality:develop Jun 28, 2024
30 checks passed
@rtibbles rtibbles deleted the quiz_reports branch June 28, 2024 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
APP: Learn Re: Learn App (content, quizzes, lessons, etc.) DEV: frontend TODO: needs review Waiting for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Quiz review - Missing selection effect, incorrect question titles
3 participants