Page MenuHomePhabricator

Fix event handler
ClosedPublic

Authored by max on Jul 28 2019, 3:26 AM.

Details

Reviewers
schrockn
bengotow
Group Reviewers
Restricted Project
Commits
R1:e1d5b2f70896: Fix event handler
Summary

Necromancy, but in this case I think the alternative of extending WindowEventMap is inappropriate.

Test Plan

Manual

Diff Detail

Repository
R1 dagster
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

max created this revision.Jul 28 2019, 3:26 AM
schrockn accepted this revision.Jul 28 2019, 1:00 PM

Confirmed that this works client-side. However let's add an issue to track this, mark it explicitly as necromancy, and assign said issue to @bengotow to audit

This revision is now accepted and ready to land.Jul 28 2019, 1:00 PM
This revision was automatically updated to reflect the committed changes.
bengotow added inline comments.Aug 5 2019, 8:26 PM
js_modules/dagit/src/runs/LogsScrollingTableHeader.tsx
83

Hey! So this onMouseMove event listener is actually attached directly to the document when the mouse down event occurs, and removed on mouseup. It's at the document level because we want the mouse movement to be tracked even if you move the mouse outside the element you're dragging (which happens anytime the drag framerate is low).

Because it's bound directly to the window I think the right type for evt: Event is MouseEvent? That might eliminate the need for the cast because MouseEvent has a screenX.