search

Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
4.02k stars 3.01k forks source link

[HOLD for payment 2025-02-05] [$250] Optimize hover state management #55652

Closed mountiny closed 2 days ago

mountiny commented 2 weeks ago

Coming from here

BACKGROUND

The ActiveHoverable component manages hover states for interactive elements in our web interface. The current implementation is using multiple event listeners, including a global document-level mouseover listener, which is causing performance overhead, especially in views with many hoverable elements. For example, when hovering over chat messages, the CPU utilization can reach 100%.

PROBLEM

  1. Global mouseover event listener on document level is causing continuous event processing and being triggered on extremely frequently (on every mouse pixel move)
  2. Each hoverable instance adds its own document listener, multiplying the overhead
  3. High CPU usage during mouse movement across the page
  4. Redundant hover state checks and updates

SOLUTION

Optimize hover state management and CPU performance by:

  1. Removing the global document-level mouseover listener
  2. Using component-level mouse events only
  3. Implementing efficient state tracking with lastKnownHoverState
    Upwork Automation - Do Not Edit
    • Upwork Job URL: https://www.upwork.com/jobs/~021882410245975337736
    • Upwork Job ID: 1882410245975337736
    • Last Price Increase: 2025-01-23
    • Automatic offers:
      • brunovjk | Reviewer | 105822890
Issue OwnerCurrent Issue Owner: @johncschuster
melvin-bot[bot] commented 2 weeks ago

Job added to Upwork: https://www.upwork.com/jobs/~021882410245975337736

melvin-bot[bot] commented 2 weeks ago

Triggered auto assignment to @johncschuster (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

melvin-bot[bot] commented 2 weeks ago

Triggered auto assignment to Contributor-plus team member for initial proposal review - @brunovjk (External)

zirgulis commented 2 weeks ago

Hi I'm Povilas from Callstack and I would like to work on this issue

melvin-bot[bot] commented 2 weeks ago

📣 @brunovjk 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job

brunovjk commented 1 week ago

This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. - https://github.com/Expensify/App/pull/54744#issuecomment-2613363088

melvin-bot[bot] commented 1 week ago

Reviewing label has been removed, please complete the "BugZero Checklist".

melvin-bot[bot] commented 1 week ago

The solution for this issue has been :rocket: deployed to production :rocket: in version 9.0.90-6 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2025-02-05. :confetti_ball:

For reference, here are some details about the assignees on this issue:

melvin-bot[bot] commented 1 week ago

@brunovjk @johncschuster @brunovjk The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

johncschuster commented 1 week ago

BugZero Checklist:

Bug classification Source of bug: - [ ] 1a. Result of the original design (eg. a case wasn't considered) - [ ] 1b. Mistake during implementation - [ ] 1c. Backend bug - [ ] 1z. Other: Where bug was reported: - [ ] 2a. Reported on production (eg. bug slipped through the normal regression and PR testing process on staging) - [ ] 2b. Reported on staging (eg. found during regression or PR testing) - [ ] 2d. Reported on a PR - [ ] 2z. Other: Who reported the bug: - [ ] 3a. Expensify user - [ ] 3b. Expensify employee - [ ] 3c. Contributor - [ ] 3d. QA - [ ] 3z. Other:
Regression Test Proposal Template - [ ] **[BugZero Assignee]** Create a GH issue for creating/updating the regression test once above steps have been agreed upon. Link to issue: ## Regression Test Proposal ### Precondition: - ### Test: 1. Do we agree 👍 or 👎
brunovjk commented 1 week ago

I will complete the checklist the day before payday :D

brunovjk commented 3 days ago

BugZero Checklist:

Bug classification Source of bug: - [x] 1a. Result of the original design (eg. a case wasn't considered) - [ ] 1b. Mistake during implementation - [ ] 1c. Backend bug - [ ] 1z. Other: Where bug was reported: - [ ] 2a. Reported on production (eg. bug slipped through the normal regression and PR testing process on staging) - [ ] 2b. Reported on staging (eg. found during regression or PR testing) - [ ] 2d. Reported on a PR - [x] 2z. Other: https://expensify.slack.com/archives/C05LX9D6E07/p1736326697079059 Who reported the bug: - [ ] 3a. Expensify user - [x] 3b. Expensify employee - [ ] 3c. Contributor - [ ] 3d. QA - [ ] 3z. Other:

Regression Test Proposal

Test:

  1. Open a chat with multiple messages
  2. Hover over different messages rapidly
  3. Verify that CPU usage remains stable and does not spike to 100%
  4. Verify that hover states are applied and removed correctly without delay

Do we agree 👍 or 👎

johncschuster commented 2 days ago

Payment Summary

Contributor: @zirgulis does not require payment (Contractor) Contributor+: @brunovjk paid $250 via Upwork