• 
      

    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)