fix: Align answer submission notification to bottom of viewport if out of view

[arjunsinghy96]

Description

This PR changes the alignment of answer submission notification. Currently the notification is aligned vertically centred if not in viewport. This behaviour was observed in Chrome browser (87+). The alignment was correct when tested on Mozilla Firefox (86.0)

JIRA Tickets: BB-3828

Sandbox URL: TBD

Screenshots

Before

After:

Testing instructions

1. Get the master devstack running
2. Sign in to LMS
3. Open Exam section of Demo Course.
4. Answer questions and click on Submit button.
5. Verify that the notification is scrolled to the centre of the viewport when not in viewport.
6. With notification in centre of viewport, submit another answer.
7. Verify that page does not scroll.
8. Now checkout to this branch and perform the same tests after restarting the LMS server
   a. Verify that the notification is scrolled to the bottom of the viewport when not in viewport.
   b. With notification in viewport, submit another answer
   c. Verify that the page does not scroll.

Deadline

None

Reviewers

• @viadanna
• @Agrendalath
• edx-reviewer

[openedx-webhooks]

Thanks for the pull request, @arjunsinghy96! I've created OSPR-5673 to keep track of it in JIRA, where we prioritize reviews. Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket:

• supporting documentation
• Open edX discussion forum threads
• timeline information ("this must be merged by XX date", and why that is)
• partner information ("this is a course on edx.org")
• any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

[Agrendalath]

jenkins run a11y

[viadanna]

jenkins run a11y

[natabene]

@arjunsinghy96 Thank you for your contribution. Please let me know once it is ready for our review.

[arjunsinghy96]

jenkins run a11y

[Agrendalath]

@arjunsinghy96, I found some typos. I haven't checked the code.
cc: @viadanna

[arjunsinghy96]

@natabene Thanks for the patience. This PR is now ready for edx review.

[viadanna]

Good to go 👍

• I tested this on my devstack.
• I read through the code.

[natabene]

@sarina This is ready for your team to review.

[davidjoy]

It's a small thing, but if you reverse the clauses in this conditional then you won't do the viewport calculation for anything except the notification-btn. If there are a lot of elements, it could reduce the work being done here.

if (elem.classList.contains('notification-btn') && !isInViewport(elem)) {

That'll short circuit if the element's class list doesn't contain notification-btn, avoiding the call to isInViewport.

[davidjoy]

Do you have a sense of how often this focus function is called? It seems like we're overriding it for all calls on this page... I'd just want to sanity check that we're not adding a whole bunch of processing to the page when we loop through this.each below.

[arjunsinghy96]

@davidjoy Thanks for the review and suggestion. I am working on it this week.

Do you have a sense of how often this focus function is called?

I will try to get some metric on this and update you by end of the week.

[arjunsinghy96]

@davidjoy Following is the frequency of focus function calls:

1. focus is called twice when rendering the sequence i.e by Sequence.render. It is called on first render of page as well as whenever the active sequence is changed by clicking on Next, Previous or the sequence button.
2. focus is called twice for each Problem in the xblock when rendering. So, if the sequence has n problems, focus will be called n*2 times.
3. focus is called once when rendering discussions on the sequence (xblock).
4. focus is called twice for videos in the sequence (xblock)
5. For staff users, focus is called once when rendering the staff toolbar on top of the page.

Additionally, focus is called twice by calculator.js on first render of the page.

Please let me know you further thoughts.

---

I tried attaching a delegated (ref) event handler on .notification-btn but the browser behaviour was preceded by the event handler callback. The notification was already moved to centre of page before the callback function ran.

[davidjoy]

Had a suggestion and a question. In general this looks like an improvement, just want to be cautious of any overhead we're adding!

[arch-bom-gocd-alerts]

📣 💥 Heads-up: You must either rebase onto master or merge master into your branch to avoid breaking the build.

We recently removed diff-quality and introduced lint-amnesty. This means that the automated quality check that has run on your branch doesn't work the same way it will on master. If you have introduced any quality failures, they might pass on the PR but then break the build on master.

This branch has been detected to not have commit 2e33565 as an ancestor. Here's how to see for yourself:

git merge-base --is-ancestor 2e335653 arjun/bb-3828-alignment && echo "You're all set" || echo "Please rebase onto master or merge master to your branch"

If you have any questions, please reach out to the Architecture team (either #edx-shared-architecture on Open edX Slack or #architecture on edX internal).

[edx-status-bot]

Your PR has finished running tests. There were no failures.

[davidjoy]

This looks good - I tested it and it's a definite improvement. 👍🏻

Waiting for checks to finish and will then merge it.

[openedx-webhooks]

@arjunsinghy96 🎉 Your pull request was merged!

Please take a moment to answer a two question survey so we can improve your experience in the future.

[edx-pipeline-bot]

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

[edx-pipeline-bot]

EdX Release Notice: This PR has been deployed to the production environment.

[edx-pipeline-bot]

EdX Release Notice: This PR has been rolled back from the production environment.

[edx-pipeline-bot]

EdX Release Notice: This PR has been deployed to the production environment.