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

Diff:

Revision 4 (+100 -82)

Show changes

Checks run (2 succeeded)

flake8 passed.
JSHint passed.
Loading...