Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop mousedown event propagation in the Focuser #10575

Merged
merged 1 commit into from
Jun 18, 2024
Merged

Stop mousedown event propagation in the Focuser #10575

merged 1 commit into from
Jun 18, 2024

Conversation

hbenl
Copy link
Contributor

@hbenl hbenl commented Jun 18, 2024

The Focuser is overlaid over the Timeline and stops all mouse events from propagating to it except mousedown events, those confused the Timeline so that it thought that a click on the "Save" or "Cancel" buttons of the FocusModePopout was a click on the timeline and so it jumped to the end of the recording.

@hbenl hbenl requested review from bvaughn and Andarist June 18, 2024 15:09
@hbenl hbenl self-assigned this Jun 18, 2024
Copy link

vercel bot commented Jun 18, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
devtools ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 18, 2024 3:09pm

Copy link

replay-io bot commented Jun 18, 2024

E2E Tests

Status Complete ↗︎
Commit 90d3966
Results
⚠️ 4 Flaky
  • authenticated/passport-01: Time travel (Replay 1, Replay 2)
  • logpoints-05: should auto-complete based on log point location (Replay 1, Replay 2)
  • logpoints-06: should be temporarily disabled (Replay 1, Replay 2)
  • object_preview-02: should allow objects in scope to be inspected (Replay 1, Replay 2)
  • 102 Passed
  • authenticated/comments-01: Test add, edit, and delete comment functionality
  • authenticated/comments-02: Test shared comments and replies (Replay 1, Replay 2)
  • authenticated/comments-03: Comment previews
  • authenticated/logpoints-01: Shared logpoints functionality (Replay 1, Replay 2)
  • authenticated/passport-02: Infrared inspection
  • authenticated/passport-03: Swiss army knife
  • authenticated/passport-04: Multiplayer
  • breakpoints-01: Test basic breakpoint functionality
  • breakpoints-02: Test unhandled divergence while evaluating at a breakpoint
  • breakpoints-03: Test stepping forward through breakpoints when rewound before the first one
  • breakpoints-04: catch, finally, generators, and async/await
  • breakpoints-05: Test interaction of breakpoints with debugger statements
  • breakpoints-06: Test log point in a sourcemapped file
  • breakpoints-07: rewind and seek using command bar and console messages
  • breakpoints-08: should be temporarily disabled
  • console_async: support console evaluations in async frames
  • console_dock: Should show the correct docking behavior for recordings with video
  • console_errors: Test that errors and warnings from various sources are shown in the console
  • console_eval: support console evaluations
  • console_warp-01: should support warping to console messages
  • console_warp-02: support pausing, warping, stepping and evaluating console messages
  • console-expressions-01: should cache input eager eval and terminal expressions per instance
  • cypress-01: Basic Test Suites panel functionality
  • cypress-02: Test Step timeline behavior
  • cypress-03: Test Step interactions
  • cypress-04: Test Step buttons and menu item
  • cypress-05: Test DOM node preview on user action step hover
  • deleted-recording: Show error message for deleted recording
  • elements-search: Element panel should support basic and advanced search modes
  • fe-1875 :: verify that steps go to the right point in time
  • file-search-01: should search files
  • focus_mode-01: should filter messages as regions based on the active focus mode
  • highlighter: element highlighter works everywhere
  • inspector-computed-01: Basic computed styles can be viewed
  • inspector-computed-02: Complex computed styles can be viewed
  • inspector-elements-01: Basic DOM tree node display
  • inspector-elements-02_node-picker: element picker and iframe behavior
  • inspector-elements-03: Nested node picker and selection behavior
  • inspector-elements-04: Keyboard shortcuts should select the right DOM nodes
  • inspector-elements-05_search: element picker and iframe behavior
  • inspector-rules-01: Basic CSS rules should be viewed
  • inspector-rules-03: Shorthand CSS rules should be viewed
  • jump-to-code-01: Test basic jumping functionality
  • logpoints-01: log-points appear in the correct order and allow time warping
  • logpoints-02: conditional log-points
  • logpoints-03: should display event properties in the console
  • logpoints-04: should display exceptions in the console
  • logpoints-07: should use the correct scope in auto-complete
  • logpoints-08: should support jumping directly to a hit point via the capsule input
  • logpoints-09: should support pending edits
  • logpoints-10: too-many-points-to-find UX
  • logpoints-11: too-many-points-to-run-analysis UX
  • network-01: should filter requests by type and text
  • network-02: should show details for the selected request
  • network-03: should sync and display the current time in relation to the network requests
  • node_console_dock: Should show the correct docking behavior for recordings without video
  • node_console-01: Basic node console behavior
  • node_console-02: uncaught exceptions should show up
  • node_control_flow: catch, finally, generators, and async/await
  • node_logpoint-01: Basic node logpoints
  • node_object_preview: Showing console objects in node
  • node_quick_open_modal-01: Test basic searching functionality
  • node_spawn: Basic subprocess spawning
  • node_stepping-01: Test stepping in async frames and async call stacks
  • node_worker-01: make sure node workers don't cause crashes
  • object_preview-01: expressions in the console after time warping
  • object_preview-03: Test previews when switching between frames and stepping
  • object_preview-04: Test scope mapping and switching between generated/original sources
  • object_preview-05: Should support logging objects as values
  • object_preview-06: HTML elements
  • object_preview-07: inspect objects in the console while paused somewhere else
  • object_preview-08: should render ellipsis for collapsed objects with truncated properties
  • playwright-01: Basic Test Suites panel functionality
  • playwright-02: Test Step timeline behavior
  • playwright-03: Test Step interactions
  • playwright-04: Test Step buttons and menu item
  • playwright-05: Test DOM node previews on user action step hover
  • react_devtools-01: Basic RDT behavior
  • react_devtools-02: RDT integrations (Chromium)
  • react_devtools-03: process and display multiple React versions in page
  • react_devtools-04: Component selection is maintained when seeking to a new point
  • redux_devtools: Test Redux DevTools.
  • repaint-01: repaints the screen screen when stepping over code that modifies the DOM
  • repaint-02: repaints on hover
  • repaint-03: repaints on seek
  • repaint-04: prefers nearest (<=) paint when seeking between paints
  • repaint-05: prefers current time if pause creation failed outside of the focus window
  • repaint-06: repaints the screen screen when stepping over code that modifies the DOM
  • resizable-panels-01: Left side Toolbar and Video should be collapsible
  • restart-session: restart debugging session
  • scopes_rerender: Test that scopes are rerendered
  • session-destroyed: errors caused by session failure should bubble to the root
  • source-line-highlights: Test source line highlighting
  • sourcemap_stacktrace: Test that stacktraces are sourcemapped
  • stacking: Element highlighter selects the correct element when they overlap
  • stepping-01: Test basic step-over/back functionality
  • stepping-02: Test fixes for some simple stepping bugs
  • stepping-03: Stepping past the beginning or end of a frame should act like a step-out
  • stepping-04: Test stepping in a frame other than the top frame
  • stepping-05: Test stepping in pretty-printed code
  • stepping-06: Test stepping in async frames and async call stacks
  • stepping-07: Test quick stepping using the keyboard
  • @hbenl hbenl merged commit fdbddbf into main Jun 18, 2024
    23 checks passed
    @hbenl hbenl deleted the hbenl/pro-661 branch June 18, 2024 16:47
    markerikson added a commit that referenced this pull request Jun 28, 2024
    * Jump-to-source falls back to parent user event if marker frame cannot be found (#10561)
    
    ### [Loom overview](https://www.loom.com/share/6d697c9136274fb8892325d39283fdac)
    
    Looking at [go/r/bb82f5c3-af5c-4e20-9f1c-5efa4ee6bd29](https://app.replay.io/recording/appreplayio--bb82f5c3-af5c-4e20-9f1c-5efa4ee6bd29), I observed that the `getCypressMarkerFrame` lookup fails to find a "marker frame" because there are only two sources in the call stack– the Replay plugin itself and Cypress. That method expects to find user code in between for some reason, so this breaks the jump-to-source behavior.
    
    In this case, the source is:
    ```js
    cy.contains('Product added to cart!').should('be.visible');
    ```
    
    That gets rendered in the UI as this:
    
    ![Screenshot 2024-06-13 at 10 46 16 PM](https://github.com/replayio/devtools/assets/29597/63920c23-82c8-45b4-9bb9-29443984b3b9)
    
    Clicking on the failed _assert_ (or trying to "jump to source") would ideally jump to the `should('be.visible')` but it fails because of the "marker frame" issue I mentioned above. The idea occurred to me that in this case, jumping to the parent command (`contains('Product added to cart!')`) is probably better than nothing. (In this case, they're even on the same line, but that won't always be true.)
    
    * Add data-private attributes (#10562)
    
    * Remove [data-private] CSS style (#10563)
    
    * Reset selected test event when test changes (#10565)
    
    * Improve visual style for React/Reduxt not-loaded error messages (#10566)
    
    * Log point panel should never show 1/0 (#10568)
    
    * Fix video toggle for horizontal DEV layout (#10569)
    
    * Support multi-line print statements (#10570)
    
    Support multi-line print statements to improve readability and text editing experience.
    
    * Stop mousedown event propagation in the Focuser (#10575)
    
    * Adjusting contrast (#10572)
    
    * Always use hoverTime for the screenshot when the user is hovering (#10578)
    
    * Fix detection of async parent pauses outside of the focus window (#10573)
    
    * Revert "Improve message shown when async stack can't be loaded (#10557)"
    
    This reverts commit 585e560.
    
    * Fix detection of async parent pauses outside of the focus window
    
    * Remove LogRocket from Devtools (#10577)
    
    * Improve heuristics for finding the user frame that dispatched a Redux… (#10576)
    
    * Improve heuristics for finding the user frame that dispatched a Redux action
    
    The original logic was iterating backwards across the point stack frames looking for an applyMiddleware frame, and going backwards one more frame from there. This is problematic because I'm seeing variance in whether or not applyMiddleware actually shows up in stack frames. For some reason it shows up in Metabase recording stack traces, but not in our own E2E tests (and we're both using RTK).  That meant that the existing logic could keep right on walking backwards long past the frame it "should" have found.
    
    What I've settled on is using the logic we had that tries to identify "is this function in a Redux middleware" based on source outlines (looking for the triply-nested function signature of a middleware definition), and stopping at the first frame that appears to be not a middleware and not applyMiddleware.  This seems to be producing pretty reasonable results.
    
    * Update to a recent `breakpoints-01` recording
    
    * Force re-running the J2C routine
    
    * Add a util for verifying J2C behavior
    
    * Add a test for Redux J2C behavior
    
    * Ignore wrong lint error
    
    * Update RDT-02 test to match current example recording
    
    * Log point panel saves pending edits when removing condition (#10579)
    
    * Add option to notify user when processing is complete (#10582)
    
    * Guard against undefined value (#10581)
    
    * Source viewer font-size fixes (#10580)
    
    Print statement panel dynamic sizing bug fixes:
    - Fixed minor font-size and line-height inconsistencies for print statement panel
    - Handle edge case with empty last line (aka Shift+Enter)
    
    ![Screenshot 2024-06-21 at 10 58 36 AM](https://github.com/replayio/devtools/assets/29597/1a8e9514-1c56-4998-84e0-7b71a1d022b7)
    
    Also updated the `useSourceListCssVariables` hook to reevaluate CSS variables (width of line numbers and hit counts) when the font size preference changes.
    
    * Better view element icon (#10583)
    
    * Enable asserts (#10584)
    
    * Handle lack of `point.frame` (#10586)
    
    * Remove unused `jumpToClickEventFunctionLocation` (#10585)
    
    * Improve video cursor display, especially for Cypress tests (#10587)
    
    * Extract utils to check for user click/keyboard test steps
    
    * Simplify UserActionEventRow J2C checks
    
    * Use mouse event times for hover display for click steps
    
    * Narrow time that we show a mouse click indicator
    
    * Only show mouse clicks within the current focus window
    
    * Revert focus window filtering for mouse cursors
    
    * Update the Cypress steps test to check for hover cursor position (#10588)
    
    * Update cypress/bankaccounts.spec example
    
    * Add test for Cypress cursor positioning
    
    * Use dashed/camel-cased data names
    
    ---------
    
    Co-authored-by: Brian Vaughn <[email protected]>
    Co-authored-by: Holger Benl <[email protected]>
    Co-authored-by: Jon Bell <[email protected]>
    Co-authored-by: Domi <[email protected]>
    Co-authored-by: Mateusz Burzyński <[email protected]>
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    None yet
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants