ContextualMenu: Update mousemove behavior #4589
Conversation
…another pending mouse enter/move/leave to improve the snappiness of the updates
|
|
||
| if (!this._isScrollIdle || targetElement === this._targetWindow.document.activeElement as HTMLElement) { | ||
| if (!this._isScrollIdle || | ||
| this._enterTimerId !== undefined || |
There was a problem hiding this comment.
this._enterTimerId !== undefined [](start = 6, length = 32)
You actually do not need to do this. The following line of code will have the same affect and save you a few characters.
this._enterTimerId ||
There was a problem hiding this comment.
While it shouldn't be zero if it is a valid id, I couldn't find anything that technically says zero is not a valid return value. I don't want to make any assumptions which might make this code more brittle
There was a problem hiding this comment.
OK sure. Your choice. I am certain it cannot be Zero. I know this because I have written such code more than once and never faced issues. That code has run for many years now.
https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/setTimeout
he returned timeoutID is a positive integer value which identifies the timer created by the call to setTimeout();
There was a problem hiding this comment.
According to IEEE 754-2008 zero can be signed and is positive by default (and can sometimes be negative), that being said it doesn't make sense to risk it
Same here, the following line will do the right thing for this code and save some bytes. If (this._enterTimerId) Refers to: packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.tsx:810 in 0b996e5. [](commit_id = 0b996e5, deletion_comment = False) |
same as agove. In fact if there was a private method to clear the timout, we will same a bunch of repetitive. Refers to: packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.tsx:844 in 0b996e5. [](commit_id = 0b996e5, deletion_comment = False) |
…es not have a submenu
…-react into jspurlin/ContextualMenuUpdateMouseMoveBehavior
5 participants
Pull request checklist
$ npm run changeDescription of changes
We've gotten reports that the existing expand on hover behavior can feel sluggish. After digging in we were delaying expand/collapse (and focus) updates for on mousemove (if the element was not already focused).
This PR makes it to where we no longer reset the delay on mousemove if there is already a pending action (from mouseenter/mousemove/mouseleave)
Before:
After:
Focus areas to test
Verified that the hover behavior is more snappy while not suffering any negative side affects