Fix expanding collapsed reviews when opening links to them.

Review Request #13946 — Created June 5, 2024 and submitted

Information

Review Board
release-7.x

Reviewers

We had some code in the ReviewRequestPageView to expand any entries that
contain an element with the window's hash, but this was only looking at
children of the entry views. For review entries, the entry view's
element itself has the ID, so we weren't doing anything. The page would
scroll purely based on the browser's behavior.

This change makes it so we test both the entry's element as well as look
for any children of that element.

I've also put the actual scroll call into a requestAnimationFrame()
callback to allow the page to reflow if necessary when expanding the
entry, which helps do a better job if the entry being expanded is at the
bottom of the page.

  • Ran js-tests.
  • Opened pages with #reviewXXXX hashes in the URL and saw that the
    correct review was now expanded and scrolled to.
Summary ID
Fix expanding collapsed reviews when opening links to them.
We had some code in the ReviewRequestPageView to expand any entries that contain an element with the window's hash, but this was only looking at children of the entry views. For review entries, the entry view's element itself has the ID, so we weren't doing anything. The page would scroll purely based on the browser's behavior. This change makes it so we test both the entry's element as well as look for any children of that element. I've also put the actual scroll call into a requestAnimationFrame() callback to allow the page to reflow if necessary when expanding the entry, which helps do a better job if the entry being expanded is at the bottom of the page. Testing Done: - Ran js-tests. - Opened pages with #reviewXXXX hashes in the URL and saw that the correct review was now expanded and scrolled to.
272456ddc6e53689d5f959dd8eed20cbf4e40e40
Description From Last Updated

Can we go with the normal conditional form of ? ... and : ... on their own lines, both being …

chipx86chipx86

Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (RB) Column: 21 Error code: W083

reviewbotreviewbot
Checks run (1 failed, 1 succeeded)
flake8 passed.
JSHint failed.

JSHint

maubin
  1. Ship It!
  2. 
      
chipx86
  1. 
      
  2. Show all issues

    Can we go with the normal conditional form of ? ... and : ... on their own lines, both being aligned with the tested value?

  3. 
      
david
Review request changed
Status:
Completed
Change Summary:
Pushed to release-7.x (3622c58)