• 
      

    Fix a hidden Publish Review button on the page.

    Review Request #11544 — Created March 22, 2021 and submitted

    Information

    Review Board
    release-4.0.x

    Reviewers

    When the split Publish button was converted into the new
    rb-c-menu-button component, it exposed a pre-existing problem where
    the review draft banner (which is always present on a review request
    page) was accepting events.

    This wasn't a problem on 3.x, because we got lucky. Previously, on
    3.x through 4.0 RC1, the banner was marked as hidden with a .hidden
    class, and that enabled a visibility: hidden. This kept anything from
    really happening in that banner.

    The newer version regressed this due to a CSS rule for
    rb-c-menu-button that unconditionally set visibility: visible on the
    menu. This, along with some combination of the transform to keep it
    off-screen and possibly the position: absolute or some other rule, was
    keeping the visible menu off-screen but the events on-screen.

    There's two bugs here:

    1) rb-c-menu shouldn't have been set to visibility: visible inside
    a menu button, unless the menu was open. This CSS rule has been
    separated from another group of rules to ensure this.

    2) The review banner should have been hidden from events entirely. This
    is being handled by setting the hidden attribute, which will hide
    the element and all children, no matter what, from the render tree,
    event tree, and accessibility tree.

    Tested all menu buttons in the UI to make sure they worked correctly,
    and opened/closed correctly. Made sure they weren't able to receive
    events when closed.

    Tested that, with the above fix alone, the empty review problem was
    gone.

    Reverted that fix, and added fix #2, for the review banner's hidden state.
    Verified that also solved the problem. Verified the review properly opened
    when adding a new comment, and that publish/discard worked.

    Tested the combination of both, making sure the issue was still fixed and
    that the menu worked when the banner was open.

    JavaScript unit tests passed.

    Summary ID
    Fix a hidden Publish Review button on the page.
    When the split Publish button was converted into the new `rb-c-menu-button` component, it exposed a pre-existing problem where the review draft banner (which is always present on a review request page) was accepting events. This wasn't a problem on 3.x, because we got lucky. Previously, on 3.x through 4.0 RC1, the banner was marked as hidden with a `.hidden` class, and that enabled a `visibility: hidden`. This kept anything from really happening in that banner. The newer version regressed this due to a CSS rule for `rb-c-menu-button` that unconditionally set `visibility: visible` on the menu. This, along with some combination of the `transform` to keep it off-screen and possibly the `position: absolute` or some other rule, was keeping the visible menu off-screen but the events on-screen. There's two bugs here: 1) `rb-c-menu` shouldn't have been set to `visibility: visible` inside a menu button, unless the menu was open. This CSS rule has been separated from another group of rules to ensure this. 2) The review banner should have been hidden from events entirely. This is being handled by setting the `hidden` attribute, which will hide the element and all children, no matter what, from the render tree, event tree, and accessibility tree.
    84077c81f990cef75b9576eaf8ca7e3d162f1ce9
    chipx86
    1. 
        
    2. 
        
    david
    1. Yay!

    2. 
        
    chipx86
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to release-4.0.x (d8fb308)