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

Add a resource completion modal #8267

Merged
merged 20 commits into from
Aug 11, 2021

Conversation

MisRob
Copy link
Member

@MisRob MisRob commented Aug 9, 2021

Summary

Removes mastery snackbars in favor of new responsive completion modal.

Note that "Next resources" section at the bottom of a resource page hasn't been removed intentionally as we don't have UI for navigating to the next resource after closing the completion modal yet.

Screenshot from 2021-08-09 16-06-13 Screenshot from 2021-08-09 16-06-31 Screenshot from 2021-08-09 16-06-49 Screenshot from 2021-08-09 16-29-10

References

Reviewer guidance

  • Test for smaller resolutions
  • Test keyboard navigation
  • Test RTL

(A) Channel resource page

  1. Login
  2. Go to "Learn" -> "Channels"
  3. Navigate to a channel resource that you haven't completed yet
  4. Finish the resource (e.g. by completing watching a video)
  5. See that snackbars with points and a next resource don't appear any more and the modal is displayed instead
  6. Interact with the modal - stay on the page, close the modal, go to the next resource, click one of the recommended resources

(B) Channel resource page for anonymous user

Make sure you're logged out and follow (A) steps. In addition to that, check that the warning message is displayed instead of the point obtained on the modal after completing a resource.

(C) Class resource page

  1. Login
  2. Go to "Learn" -> "Classes"
  3. Navigate to a class resource that you haven't completed yet
  4. Finish the resource (e.g. by completing watching a video)
  5. See that snackbars with points and a next resource don't appear any more and the modal is displayed instead
  6. Interact with the modal - stay on the page, close the modal, go to the next resource, click one of the recommended resources

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

@MisRob MisRob added the TODO: needs review Waiting for review label Aug 9, 2021
@MisRob MisRob force-pushed the completion-modal branch from bbd109f to 0c3125b Compare August 9, 2021 14:40
@radinamatic
Copy link
Member

Looking good to me @MisRob! 💯

Tested as signed in and anonymous user, in RTL, and on small resolutions. Keyboard navigation works well, focus is trapped inside modal and with logical order of interactive elements. 🙂

RTL mobile RTL
LEDev2104 (start)  Running  - Oracle VM VirtualBox_018 LEDev2104 (start)  Running  - Oracle VM VirtualBox_017

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.

I've done a first read and so far it looks great, but I got to this later in the day than I was expecting and want to give it another look in the morning when my brain is fresher to be sure I'm not missing anything. But nice work @MisRob - appreciate (as always) your super clean code and all of the comments 😊

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.

Looks great, @MisRob! ✅

@marcellamaki marcellamaki merged commit 84b05ff into learningequality:develop Aug 11, 2021
@MisRob MisRob deleted the completion-modal branch September 5, 2021 10:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TODO: needs review Waiting for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants