• 
      

    Fix review handle bar not draggable on touchscreens

    Review Request #11816 — Created Sept. 17, 2021 and updated

    Information

    Review Board
    master

    Reviewers

    Review handle bar was draggable with mouse due to the mouseup, mousedown
    and mousemove events. However, there were no events to allow for
    touchscreen access and thus the handle bar was not draggable for
    touchscreen devices.

    We are now utilizing pointer events instead of mouse events to account
    for touchscreen input.

    Tested in Chrome and Firefox browser using touchscreen accessibility
    mode. Also tested mouse events continue working as expected.

    Summary ID Author
    WIP: page is not scrollable but the handle is
    e28830a1736a35dc4131d183a1956ee9ebfc5ea7 aruslanov@fispan.com
    cleaned up all the console.logs
    a6b632332a7b81f65c16466f421baba60f05eff2 aruslanov@fispan.com
    Works as expected: handle is draggable and page is scrollable
    0152e9b7d47bf7fd3f820ac6fc8886723f7eaf7e aruslanov@fispan.com
    aligned some items; deleted whitespaces
    9984298fe4465f58a1dac004c3298e15d8ae4491 aruslanov@fispan.com
    removed white space
    28268e2821865fb8d290ac763bd51714b92a21aa aruslanov@fispan.com
    Description From Last Updated

    The Testing Done is a bit too long. We need this to wrap around 75 characters so it fits well …

    chipx86chipx86

    The Description needs to be fleshed out a bit. It shoud clearly communicate to anyone reading what the problem was …

    chipx86chipx86

    The Summary is making a statement, but not describing the change particularly. Similar to the description, I recommend reading through …

    chipx86chipx86

    Blank line between new selectors. It'd also help to document this by adding a comment explaining what this is for, …

    chipx86chipx86

    It might be worth adding a comment just above this block explaining why we're doing this.

    mike_conleymike_conley

    There's some extra trailing whitespace that was left over here.

    chipx86chipx86

    With the rename, the : ... no longer aligns with ? ....

    chipx86chipx86

    With the rename, these no longer align.

    chipx86chipx86

    The extra blank line added here should be removed.

    chipx86chipx86
    akim.ruslanov
    akim.ruslanov
    chipx86
    1. 
        
    2. Show all issues

      The Testing Done is a bit too long. We need this to wrap around 75 characters so it fits well in a commit message.

    3. Show all issues

      The Description needs to be fleshed out a bit. It shoud clearly communicate to anyone reading what the problem was and how it was addressed (without going too far into the nitty-gritty).

      You can look at prior commit messages or some of our open review requests for some inspiration, and I recommend reading through our Writing Good Change Descriptions guide.

      1. I believe the description should now be a lot more clear.

    4. Show all issues

      The Summary is making a statement, but not describing the change particularly. Similar to the description, I recommend reading through our commit or review request history to see the way we describe changes.

      1. I believe summary should now describe the change/fix clearly

    5. Show all issues

      Blank line between new selectors.

      It'd also help to document this by adding a comment explaining what this is for, so that nobody has to remember or go through commit messages.

      Multi-line format for LessCSS files looks like:

      /*
       * <text>
       */
      
    6. reviewboard/static/rb/css/pages/diffviewer.less (Diff revision 2)
       
       
       
       
      Show all issues

      There's some extra trailing whitespace that was left over here.

    7. Show all issues

      With the rename, the : ... no longer aligns with ? ....

    8. Show all issues

      With the rename, these no longer align.

    9. Show all issues

      The extra blank line added here should be removed.

    10. 
        
    akim.ruslanov
    mike_conley
    1. Thanks, Akim! Is the hit area still too small, or does this seem to address that somehow?

      1. The hit area is of a proper size. The real issue was the viewport being dragged, but that is already fixed using touch-action:none

    2. reviewboard/static/rb/css/pages/diffviewer.less (Diff revision 2)
       
       
       
       
      Show all issues

      It might be worth adding a comment just above this block explaining why we're doing this.

      1. I actually just added a comment in the most recent commit

    3. 
        
    akim.ruslanov
    akim.ruslanov
    akim.ruslanov
    Review request changed
    Commits:
    Summary ID Author
    WIP: page is not scrollable but the handle is
    e28830a1736a35dc4131d183a1956ee9ebfc5ea7 aruslanov@fispan.com
    cleaned up all the console.logs
    a6b632332a7b81f65c16466f421baba60f05eff2 aruslanov@fispan.com
    Works as expected: handle is draggable and page is scrollable
    0152e9b7d47bf7fd3f820ac6fc8886723f7eaf7e aruslanov@fispan.com
    aligned some items; deleted whitespaces
    9984298fe4465f58a1dac004c3298e15d8ae4491 aruslanov@fispan.com
    WIP: page is not scrollable but the handle is
    e28830a1736a35dc4131d183a1956ee9ebfc5ea7 aruslanov@fispan.com
    cleaned up all the console.logs
    a6b632332a7b81f65c16466f421baba60f05eff2 aruslanov@fispan.com
    Works as expected: handle is draggable and page is scrollable
    0152e9b7d47bf7fd3f820ac6fc8886723f7eaf7e aruslanov@fispan.com
    aligned some items; deleted whitespaces
    9984298fe4465f58a1dac004c3298e15d8ae4491 aruslanov@fispan.com
    removed white space
    28268e2821865fb8d290ac763bd51714b92a21aa aruslanov@fispan.com
    Branch:
    release-4.0.x
    master

    Checks run (2 succeeded)

    flake8 passed.
    JSHint passed.