From 17e344c552f69e58c512d923472f60e06af5ba12 Mon Sep 17 00:00:00 2001 From: Julien Wajsberg Date: Wed, 30 Nov 2022 15:31:23 +0100 Subject: [PATCH] Prevent ctrl+wheel events in timeline --- src/components/shared/Reorderable.js | 14 +++- src/components/timeline/ActiveTabTimeline.js | 12 +++- src/components/timeline/FullTimeline.js | 15 ++++- src/components/timeline/OriginsTimeline.js | 42 +++++++----- src/components/timeline/index.js | 67 +++++++++++++++++++- 5 files changed, 122 insertions(+), 28 deletions(-) diff --git a/src/components/shared/Reorderable.js b/src/components/shared/Reorderable.js index 752bb88482..309a2acbd2 100644 --- a/src/components/shared/Reorderable.js +++ b/src/components/shared/Reorderable.js @@ -27,6 +27,9 @@ type Props = {| // See https://flow.org/en/docs/react/children/ for more information. // Be careful: children need to handle a `style` property. children: React.ChildrenArray>, + // If present, this will be attached to the container added for these + // children. As a reminder, the container will use the tagName defined above. + innerElementRef?: React.Ref, |}; type State = {| @@ -240,7 +243,7 @@ export class Reorderable extends React.PureComponent { } render() { - const { className, order } = this.props; + const { className, order, innerElementRef } = this.props; const children = React.Children.toArray(this.props.children); const orderedChildren = order.map((childIndex) => children[childIndex]); const TagName = this.props.tagName; @@ -248,7 +251,11 @@ export class Reorderable extends React.PureComponent { if (this.state.phase === 'RESTING') { return ( - + {orderedChildren} ); @@ -261,8 +268,9 @@ export class Reorderable extends React.PureComponent { adjustPrecedingBy, adjustSucceedingBy, } = this.state; + return ( - + {orderedChildren.map((child, childIndex) => { const style = { transition: '200ms ease-in-out transform', diff --git a/src/components/timeline/ActiveTabTimeline.js b/src/components/timeline/ActiveTabTimeline.js index 02cc5cabcc..c848698460 100644 --- a/src/components/timeline/ActiveTabTimeline.js +++ b/src/components/timeline/ActiveTabTimeline.js @@ -33,6 +33,11 @@ import type { import type { ConnectedProps } from 'firefox-profiler/utils/connect'; +type OwnProps = {| + // This ref will be added to the inner container. + +innerElementRef?: React.Ref, +|}; + type StateProps = {| +committedRange: StartEndRange, +globalTracks: ActiveTabGlobalTrack[], @@ -43,7 +48,7 @@ type StateProps = {| type Props = {| ...SizeProps, - ...ConnectedProps<{||}, StateProps, {||}>, + ...ConnectedProps, |}; type State = {| @@ -85,6 +90,7 @@ class ActiveTabTimelineImpl extends React.PureComponent { panelLayoutGeneration, globalTracks, globalTrackReferences, + innerElementRef, } = this.props; return ( @@ -102,7 +108,7 @@ class ActiveTabTimelineImpl extends React.PureComponent { initialSelected={this.state.initialSelected} forceLayoutGeneration={this.state.forceLayoutGeneration} > -
    +
      {globalTracks.map((globalTrack, trackIndex) => ( { } } -export const ActiveTabTimeline = explicitConnect<{||}, StateProps, {||}>({ +export const ActiveTabTimeline = explicitConnect({ mapStateToProps: (state) => ({ globalTracks: getActiveTabGlobalTracks(state), globalTrackReferences: getActiveTabGlobalTrackReferences(state), diff --git a/src/components/timeline/FullTimeline.js b/src/components/timeline/FullTimeline.js index afe545fe90..f9ee35a5a0 100644 --- a/src/components/timeline/FullTimeline.js +++ b/src/components/timeline/FullTimeline.js @@ -47,6 +47,11 @@ import type { import type { ConnectedProps } from 'firefox-profiler/utils/connect'; +type OwnProps = {| + // This ref will be added to the inner container. + +innerElementRef?: React.Ref, +|}; + type StateProps = {| +committedRange: StartEndRange, +globalTracks: GlobalTrack[], @@ -64,7 +69,7 @@ type DispatchProps = {| type Props = {| ...SizeProps, - ...ConnectedProps<{||}, StateProps, DispatchProps>, + ...ConnectedProps, |}; type State = {| @@ -144,6 +149,7 @@ class FullTimelineImpl extends React.PureComponent { panelLayoutGeneration, hiddenTrackCount, changeRightClickedTrack, + innerElementRef, } = this.props; return ( @@ -173,6 +179,7 @@ class FullTimelineImpl extends React.PureComponent { order={globalTrackOrder} orient="vertical" onChangeOrder={changeGlobalTrackOrder} + innerElementRef={innerElementRef} > {globalTracks.map((globalTrack, trackIndex) => ( { } } -export const FullTimeline = explicitConnect<{||}, StateProps, DispatchProps>({ +export const FullTimeline = explicitConnect< + OwnProps, + StateProps, + DispatchProps +>({ mapStateToProps: (state) => ({ globalTracks: getGlobalTracks(state), globalTrackOrder: getGlobalTrackOrder(state), diff --git a/src/components/timeline/OriginsTimeline.js b/src/components/timeline/OriginsTimeline.js index b87d6f954f..fe3cba7c36 100644 --- a/src/components/timeline/OriginsTimeline.js +++ b/src/components/timeline/OriginsTimeline.js @@ -37,6 +37,11 @@ import type { ConnectedProps } from 'firefox-profiler/utils/connect'; import './OriginsTimeline.css'; +type OwnProps = {| + // This ref will be added to the inner container. + +innerElementRef?: React.Ref, +|}; + type StateProps = {| +committedRange: StartEndRange, +panelLayoutGeneration: number, @@ -51,7 +56,7 @@ type DispatchProps = {| type Props = {| ...SizeProps, - ...ConnectedProps<{||}, StateProps, DispatchProps>, + ...ConnectedProps, |}; type State = {| @@ -145,6 +150,7 @@ class OriginsTimelineView extends React.PureComponent { width, panelLayoutGeneration, originsTimeline, + innerElementRef, } = this.props; return ( @@ -161,7 +167,7 @@ class OriginsTimelineView extends React.PureComponent { panelLayoutGeneration={panelLayoutGeneration} initialSelected={this.state.initialSelected} > -
        +
          {originsTimeline.map(this.renderTrack)}
        @@ -171,18 +177,20 @@ class OriginsTimelineView extends React.PureComponent { } } -export const TimelineOrigins = explicitConnect<{||}, StateProps, DispatchProps>( - { - mapStateToProps: (state) => ({ - threads: getThreads(state), - committedRange: getCommittedRange(state), - zeroAt: getZeroAt(state), - panelLayoutGeneration: getPanelLayoutGeneration(state), - originsTimeline: getOriginsTimeline(state), - }), - mapDispatchToProps: { - changeSelectedThreads, - }, - component: withSize(OriginsTimelineView), - } -); +export const TimelineOrigins = explicitConnect< + OwnProps, + StateProps, + DispatchProps +>({ + mapStateToProps: (state) => ({ + threads: getThreads(state), + committedRange: getCommittedRange(state), + zeroAt: getZeroAt(state), + panelLayoutGeneration: getPanelLayoutGeneration(state), + originsTimeline: getOriginsTimeline(state), + }), + mapDispatchToProps: { + changeSelectedThreads, + }, + component: withSize(OriginsTimelineView), +}); diff --git a/src/components/timeline/index.js b/src/components/timeline/index.js index 19965139d9..3f342538d5 100644 --- a/src/components/timeline/index.js +++ b/src/components/timeline/index.js @@ -22,15 +22,76 @@ type StateProps = {| type Props = ConnectedProps<{||}, StateProps, {||}>; class TimelineImpl extends React.PureComponent { + // This may contain a function that's called whenever we want to remove the + // "wheel" listener. + _removeWheelListener: null | (() => mixed) = null; + + // This effectively disable the pinch-to-zoom as well as ctrl+mousewheel + // gestures. Indeed in the timeline it is confusing. In the future we'll want + // to couple this with the preview selection like in the Viewport HOC. + preventPinchToZoom(e: WheelEvent) { + if (e.ctrlKey) { + e.preventDefault(); + } + } + + // This will be registered as a ref property to the DOM element displaying the + // tracks. We use this solution of registering the wheel event in this ref + // listener because: + // * we can't use React's event handling, because it doesn't allow us to use + // the "passive: false" way of registering the event handler. But we need this + // if we want to be able to prevent the default action of page zooming. + // * we want to be sure to register it whenever the element changes. + _onTimelineMountWithRef = (ref: HTMLElement | null) => { + if (this._removeWheelListener) { + this._removeWheelListener(); + this._removeWheelListener = null; + } + + if (!ref) { + return; + } + + // without pinning to a const variable, Flow isn't sure that we don't change + // the `ref` variable in some of the function calls below, and therefore + // that it won't be null. + const existingRef = ref; + + // Disable pinch-to-zoom and ctrl + wheel otherwise, on the timeline. + // Indeed the users are used to this gesture to zoom in in our charts, and + // may use the same gesture elsewhere because of their habits, however this + // doesn't do what they expect and instead zooms in the page, which is distracting. + existingRef.addEventListener('wheel', this.preventPinchToZoom, { + passive: false, + }); + + this._removeWheelListener = () => { + existingRef.removeEventListener('wheel', this.preventPinchToZoom, { + passive: false, + }); + }; + }; + + componentWillUnmount() { + if (this._removeWheelListener) { + this._removeWheelListener(); + this._removeWheelListener = null; + } + } + render() { const { timelineTrackOrganization } = this.props; switch (timelineTrackOrganization.type) { case 'full': - return ; + return ; case 'active-tab': - return ; + return ( + + ); case 'origins': - return ; + return ( + + ); default: throw assertExhaustiveCheck( timelineTrackOrganization,