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

Handle edge cases for resource selection in quizzes #12444

Merged
merged 2 commits into from
Jul 17, 2024

Conversation

rtibbles
Copy link
Member

Summary

  • Ensure that a single exercise can always be selected when no exercises are selected.
  • More prominently show the max selectable questions warning.

References

Fixes #12443

Reviewer guidance

Navigate to a folder in a channel that only has exercises that have more than 10 questions.
Change the number of questions to be selected to "1".
See that each individual resource is still selectable, but that select all is not shown, and the warning text is displayed.
Select one resource.
See that all other resources are now disabled for selection.
Scroll down through the folder, and see that the warning text is displayed 'stickily', so that the warning is displayed prominently. (Note, this may not work on all browsers, but is intended as a progressive enhancement where supported).


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

…s are selected.

More prominently show the max selectable questions warning.
@github-actions github-actions bot added APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) DEV: frontend labels Jul 15, 2024
@pcenov
Copy link
Member

pcenov commented Jul 16, 2024

Hi @rtibbles I've confirmed that this is working correctly now in Chrome, Firefox, Edge and Safari. My only problem is that the max selectable questions warning says '10 questions or fewer' while at the bottom of the page we see '17 questions selected':

2024-07-16_13-55-20

@radinamatic
Copy link
Member

Agreed, confusing indeed...

We may know that this actually translates to:

You selected (1 resource with) 17 questions of which 1 will be added (because you specified in the field you want to add just 1)

However, the way these numbers are presented in the UI, plus the alert about max number, just leaves the user 🤔

@marcellamaki
Copy link
Member

Maybe we remove the warning banner for the edge case? I realize this isn't what we had previously concluded Richard... I honestly am not sure what the best path here is. I agree that it's confusing, and that we're somewhat constrained with finding a "least confusing" option for release.

@rtibbles
Copy link
Member Author

I think the simplest solution is merely to make the warning banner a max of: number of selected questions, and our maximum. Hiding the banner would be a problem, as it would then remove the explanation as to why no other resources are selectable.

@marcellamaki
Copy link
Member

works for me

@rtibbles
Copy link
Member Author

🎉

Screencast.from.07-16-2024.10.57.14.AM.webm

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.

LGTM!

@rtibbles rtibbles merged commit 9f86d23 into learningequality:release-v0.17.x Jul 17, 2024
33 checks passed
@rtibbles rtibbles deleted the one_ping_only branch July 17, 2024 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) DEV: frontend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants