Bug 2019800 - Avoid unnecessary scrolling upon job clicks

[Rob--W]

Fixes https://bugzilla.mozilla.org/show_bug.cgi?id=2019800 (see bug for analysis).

The bug was introduced 7 years ago by #5293 .
Surprisingly, the selectors for querySelector from the version of the code before PR 5293 are still valid, and are being re-introduced by this patch.

[Rob--W]

CI is red but that has nothing to do with my changes. All other recent PRs have the same error.

Details

> treeherder@ lint /home/circleci/project
> biome lint

biome.json:2:14 deserialize ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

  i The configuration schema version does not match the CLI version 2.3.15
  
    1 │ {
  > 2 │   "$schema": "https://biomejs.dev/schemas/2.3.10/schema.json",
      │              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    3 │   "vcs": {
    4 │     "enabled": true,
  
  i   Expected:                     2.3.15
      Found:                        2.3.10
  
  
  i Run the command biome migrate to migrate the configuration file.
  

ui/job-view/App.jsx:190:48 lint/correctness/useHookAtTopLevel ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

  × This hook is being called from a nested function, but all hooks must be called unconditionally from the top-level component.
  
    188 │   }, [navigate]);
    189 │ 
  > 190 │   const handleStorageEvent = flushSync(() => { useCallback((e) => {
        │                                                ^^^^^^^^^^^
    191 │     if (e.key === 'user') {
    192 │       setUser(JSON.parse(e.newValue) || { isLoggedIn: false, isStaff: false });
  
  i For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
  
  i See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
  

Checked 414 files in 191ms. No fixes applied.
Found 1 error.
Found 1 info.
lint ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

  × Some errors were emitted while running checks.
  

 ELIFECYCLE  Command failed with exit code 1.

Exited with code exit status 1

[Rob--W]

The original version of the patch is not good enough to fully fix the reported issue. To figure out why the element kept scrolling upon clicking the button, I intercepted the scrollIntoView method as follows:

{
  let es = Element.prototype.scrollIntoView;
  Element.prototype.scrollIntoView = function (...a) {
  debugger;
  return es.apply(this, a);
  };
}

Then I clicked on the button while the DevTools was open in a separate window. The breakpoint triggered here:

treeherder/ui/hooks/useJobButtonRegistry.js

Lines 128 to 129 in d43d586

	if (element && typeof element.scrollIntoView === 'function') {
	element.scrollIntoView({ behavior: 'smooth', block: 'center' });

That logic appears to have been added in #9180, without prior history for the logic. I think that the most sensible thing to do here is to call the fixed version that I introduced in this PR.

[Rob--W]

Unit tests appear to fail, suggesting that #details-panel does not always exist. This failure is a surprise to me because I thought that details-panel always exists. I'll try falling back to window.innerHeight - it does not hurt, and in the worst case we zoom in more often than needed.

Output of test run:

Details

FAIL tests/ui/job-view/Filtering_test.jsx (6.986 s)
  ● Filtering › by keywords › KeyboardShortcut f: focus the quick filter input

    TypeError: Cannot read properties of null (reading 'getBoundingClientRect')

      22 |   // The details view, always shown at the bottom.
      23 |   const bottomPanelElem = document.querySelector("#details-panel");
    > 24 |   const bottomBound = bottomPanelElem.getBoundingClientRect().top;
         |                                       ^
      25 |
      26 |   return offset.top >= topBound && offset.bottom <= bottomBound;
      27 | };

      at getBoundingClientRect (ui/helpers/scrollToElement.js:24:39)
      at isOnScreen (ui/helpers/scrollToElement.js:32:8)
      at ui/hooks/useJobButtonRegistry.js:129:26
      at invokeTheCallbackFunction (node_modules/.pnpm/jsdom@20.0.3/node_modules/jsdom/lib/jsdom/living/generated/Function.js:19:26)
      at runAnimationFrameCallbacks (node_modules/.pnpm/jsdom@20.0.3/node_modules/jsdom/lib/jsdom/browser/Window.js:603:13)
      at Timeout._onTimeout (node_modules/.pnpm/jsdom@20.0.3/node_modules/jsdom/lib/jsdom/browser/Window.js:581:11)