Remove visual and interaction issues in InlineEditorView.

Review Request #13206 — Created Aug. 12, 2023 and submitted

Information

Review Board
release-6.x

Reviewers

There have been two long-standing issues in our inline editors that have
impacted users in various ways over the years.

First, the act of hiding the edit icon could affect layout by a pixel or
two, depending on its container. We've previously attempted to account
for this by making sure the container with the edit icon was a
sufficient height, but this still didn't fully solve it.

To address this, we no longer outright hide the icon using display.
Instead, we set visibility. This ensures the icon's bounding boxes
remain, preventing any and all layout issues.

Second, sometimes when attempting to click a text field to edit it, the
editor would not open. A second click usually did the trick. This had
the effect of making the editor feel unresponsive.

The reason this happened was that we tried to prevent opening if there
was anything resembling a drag (mousemove in-between a mousedown
and mouseup). This had some problems, though. From testing, it was
possible to get a mousemove that didn't even report different
coordiantes from the mousedown. Also, it's not uncommon to simply move
the mouse slightly when clicking/releasing the mouse button or pressing
down/up on the trackpad.

We unfortunately can't use dragstart to let the browser decide what's
a drag and what's not, since this requires that we mark the editor as
draggable. So instead, we watch the mouse events and see if there's
sufficient movement away from the starting coordinates (over 3px in any
direction). If it's over 3 pixels, we consider it a drag nad cancel
going into edit mode. If it's less, we allow edit mode.

Tested going into and out of edit mode in some cases where I was noticing
sub-pixel jumps. Verified that those were gone.

Tested arbitrarily clicking around on text fields to go into edit mode.
It was reliable 100% of the time.

Tested dragging text and links, and had no trouble.

Tested very carefully dragging just one, two, three, and four pixels at
a time, verifying that the threshold was being met.

Summary ID
Remove visual and interaction issues in InlineEditorView.
There have been two long-standing issues in our inline editors that have impacted users in various ways over the years. First, the act of hiding the edit icon could affect layout by a pixel or two, depending on its container. We've previously attempted to account for this by making sure the container with the edit icon was a sufficient height, but this still didn't fully solve it. To address this, we no longer outright hide the icon using `display`. Instead, we set `visibility`. This ensures the icon's bounding boxes remain, preventing any and all layout issues. Second, sometimes when attempting to click a text field to edit it, the editor would not open. A second click usually did the trick. This had the effect of making the editor feel unresponsive. The reason this happened was that we tried to prevent opening if there was anything resembling a drag (`mousemove` in-between a `mousedown` and `mouseup`). This had some problems, though. From testing, it was possible to get a `mousemove` that didn't even report different coordiantes from the `mousedown`. Also, it's not uncommon to simply move the mouse slightly when clicking/releasing the mouse button or pressing down/up on the trackpad. We unfortunately can't use `dragstart` to let the browser decide what's a drag and what's not, since this requires that we mark the editor as draggable. So instead, we watch the mouse events and see if there's sufficient movement away from the starting coordinates (over 3px in any direction). If it's over 3 pixels, we consider it a drag nad cancel going into edit mode. If it's less, we allow edit mode.
fbc2e5df77c3b4c8653d81d398aa95721bd2529b
Description From Last Updated

Can we change "set this" to "default this"?

daviddavid
david
  1. 
      
  2. Can we change "set this" to "default this"?

  3. 
      
chipx86
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-6.x (965b225)
Loading...