Skip to content

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

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.

Sign up for GitHub

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

Merged
davidjoy merged 1 commit into from
Jun 30, 2021
Merged

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

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions lms/templates/courseware/courseware.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,48 @@
</script>
% endif

<script type="text/javascript">
/* Helper function isInViewport checks whether
the given element is in viewport vertically or not */
function isInViewport(el) {
const scroll = window.scrollY || window.pageYOffset
const elementTop = el.getBoundingClientRect().top + scroll
const viewport = {
top: scroll,
bottom: scroll + window.innerHeight,
}
const elementMid = elementTop + el.clientHeight/2
// Returns true if the middle of the element is in the viewport.
return elementMid >= viewport.top && elementMid <= viewport.bottom
}

/* Add a jQuery plugin to override the focus behavior.
When focused, if the element in not in the viewport then
the element will be scrolled to the bottom of the viewport.
*/
(function ($) {
Copy link
Contributor

@davidjoy davidjoy Jun 3, 2021

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@arjunsinghy96 arjunsinghy96 Jun 15, 2021

Choose a reason for hiding this comment

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

@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.

Copy link
Contributor Author

@arjunsinghy96 arjunsinghy96 Jun 21, 2021

Choose a reason for hiding this comment

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

@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.

$.fn.extend({
focus: (function(orig) {
return function(delay, fn) {
orig.apply(this, arguments);
this.each(function(){
var elem = this;
// Scroll only when the element is not in the viewport and it contains the notification-btn.
if (elem.classList.contains('notification-btn') && !isInViewport(elem)) {
this.scrollIntoView({
behaviour: 'auto',
block: 'end',
})
}
})
return this;
}
})($.fn.focus),

})
})(jQuery)
</script>

${HTML(fragment.foot_html())}

</%block>
Expand Down