From f86567c1f22f5d4dfa9bcc097efaef3ecb8b44bc Mon Sep 17 00:00:00 2001 From: Kohei Yoshino Date: Mon, 8 Jan 2018 09:22:45 -0500 Subject: Bug 1427800 - Wrong anchor scrolling with old UI --- extensions/BugModal/web/bug_modal.js | 31 ++++++++------- extensions/BugModal/web/common_bug_modal.js | 28 +++++++------- js/global.js | 58 +++++++++++++++++++++++++---- template/en/default/bug/navigate.html.tmpl | 2 +- 4 files changed, 80 insertions(+), 39 deletions(-) diff --git a/extensions/BugModal/web/bug_modal.js b/extensions/BugModal/web/bug_modal.js index c1080cde8..c6abb5d7a 100644 --- a/extensions/BugModal/web/bug_modal.js +++ b/extensions/BugModal/web/bug_modal.js @@ -700,7 +700,7 @@ $(function() { $('#needinfo-scroll') .click(function(event) { event.preventDefault(); - $.scrollTo($('#needinfo_role'), function() { $('#needinfo_role').focus(); }); + $.scrollTo($('#needinfo_container'), function() { $('#needinfo_role').focus(); }); }); // knob @@ -830,13 +830,15 @@ $(function() { // remove embedded links to attachment details reply_text = reply_text.replace(/(attachment\s+\d+)(\s+\[[^\[\n]+\])+/gi, '$1'); - if ($('#comment').val() != reply_text) { - $('#comment').val($('#comment').val() + reply_text); - } - if (BUGZILLA.user.settings.autosize_comments) { - autosize.update($('#comment')); - } $.scrollTo($('#comment'), function() { + if ($('#comment').val() != reply_text) { + $('#comment').val($('#comment').val() + reply_text); + } + + if (BUGZILLA.user.settings.autosize_comments) { + autosize.update($('#comment')); + } + $('#comment').focus(); }); }); @@ -942,7 +944,7 @@ $(function() { $('#' + id + '-view').hide(); $('#' + id).show().focus().select(); }); - + // timetracking $('#work_time').change(function() { // subtracts time spent from remaining time @@ -1429,18 +1431,15 @@ $(function() { let $target; if (typeof target === 'string') { - $target = $('#' + target); + $target = document.getElementById(target); window.location.hash = target; } else { - $target = target; - } - - if ($target.length) { - $('main').scrollTop(Math.round($target.position().top) - 20); + // Use raw DOM node instead of jQuery + $target = target.get(0); } - if (complete) { - complete(); + if ($target) { + scroll_element_into_view($target, complete); } } diff --git a/extensions/BugModal/web/common_bug_modal.js b/extensions/BugModal/web/common_bug_modal.js index 160f922cc..dc91824f6 100644 --- a/extensions/BugModal/web/common_bug_modal.js +++ b/extensions/BugModal/web/common_bug_modal.js @@ -424,7 +424,7 @@ $(function() { var rbs = $("#readable-bug-status"); var rbs_text = bugzillaReadableStatus.readable(rbs.data('readable-bug-status')); rbs.text(rbs_text); - + if (BUGZILLA.user.id === 0) return; // @@ -700,7 +700,7 @@ $(function() { $('#needinfo-scroll') .click(function(event) { event.preventDefault(); - $.scrollTo($('#needinfo_role'), function() { $('#needinfo_role').focus(); }); + $.scrollTo($('#needinfo_container'), function() { $('#needinfo_role').focus(); }); }); // knob @@ -831,10 +831,13 @@ $(function() { // remove embedded links to attachment details reply_text = reply_text.replace(/(attachment\s+\d+)(\s+\[[^\[\n]+\])+/gi, '$1'); - if ($('#comment').val() != reply_text) { - $('#comment').val($('#comment').val() + reply_text); - } - $.scrollTo($('#comment'), function() { $('#comment').focus(); }); + $.scrollTo($('#comment'), function() { + if ($('#comment').val() != reply_text) { + $('#comment').val($('#comment').val() + reply_text); + } + + $('#comment').focus(); + }); }); // add comment --> private @@ -1356,18 +1359,15 @@ $(function() { let $target; if (typeof target === 'string') { - $target = $('#' + target); + $target = document.getElementById(target); window.location.hash = target; } else { - $target = target; - } - - if ($target.length) { - $('main').scrollTop(Math.round($target.position().top) - 20); + // Use raw DOM node instead of jQuery + $target = target.get(0); } - if (complete) { - complete(); + if ($target) { + scroll_element_into_view($target, complete); } } diff --git a/js/global.js b/js/global.js index 93f364c9e..68aceb03f 100644 --- a/js/global.js +++ b/js/global.js @@ -214,22 +214,64 @@ const detect_blocked_gravatars = () => { } /** - * If the URL contains a hash like #c10, scroll down the page to show the - * element below the fixed global header. This workaround is required for - * comments on show_bug.cgi, components on describecomponents.cgi, etc. + * If the current URL contains a hash like `#c10`, adjust the scroll position to + * make some room above the focused element. */ -const scroll_element_into_view = () => { +const adjust_scroll_onload = () => { if (location.hash) { - const $main = document.querySelector('main'); const $target = document.querySelector(location.hash); if ($target) { - window.setTimeout(() => $main.scrollTop = $target.offsetTop - 20, 50); + window.setTimeout(() => scroll_element_into_view($target), 50); } } } +/** + * Bring an element into the visible area of the browser window. Unlike the + * native `Element.scrollIntoView()` function, this adds some extra room above + * the target element. Smooth scroll can be done using CSS. + * @param {Element} $target - An element to be brought. + * @param {Function} [complete] - An optional callback function to be executed + * once the scroll is complete. + */ +const scroll_element_into_view = ($target, complete) => { + let top = 0; + let $element = $target; + + // Traverse up in the DOM tree to the scroll container of the + // focused element, either `
` or `
`. + do { + top += ($element.offsetTop || 0); + $element = $element.offsetParent; + } while ($element && !$element.matches('main, [role="feed"]')) + + if (!$element) { + return; + } + + if (typeof complete === 'function') { + const callback = () => { + $element.removeEventListener('scroll', listener); + complete(); + }; + + // Emulate the `scrollend` event + const listener = () => { + window.clearTimeout(timer); + timer = window.setTimeout(callback, 100); + }; + + // Make sure the callback is always fired even if no scroll happened + let timer = window.setTimeout(callback, 100); + + $element.addEventListener('scroll', listener); + } + + $element.scrollTop = top - 20; +} + window.addEventListener('DOMContentLoaded', focus_main_content, { once: true }); window.addEventListener('load', detect_blocked_gravatars, { once: true }); -window.addEventListener('load', scroll_element_into_view, { once: true }); -window.addEventListener('hashchange', scroll_element_into_view); +window.addEventListener('load', adjust_scroll_onload, { once: true }); +window.addEventListener('hashchange', adjust_scroll_onload); diff --git a/template/en/default/bug/navigate.html.tmpl b/template/en/default/bug/navigate.html.tmpl index 027e3fcf9..862ec7ace 100644 --- a/template/en/default/bug/navigate.html.tmpl +++ b/template/en/default/bug/navigate.html.tmpl @@ -39,7 +39,7 @@ [%# Links to more things users can do with this bug. %] [% Hook.process("links") %] -
  •  - Top of page
    • +
  •  - Top of page
    • [% PROCESS "bug/tagging.html.tmpl" %]
    -- cgit v1.2.3-24-g4f1b