fix: Control-bar autohide when cursor placed over it #5258

[xjoaoalvesx]

Description

Solves #5258

Specific Changes proposed

Adds two event handlers for 'mouseenter' and 'mouseleave' events on the control-bar.
When the 'mouseenter' is triggered the inactivityTimeout is set to 0 and on 'mouseleave' is set to the default value (2s).

Expected behavior:

• cursor stops inside control bar -> sets timeout 0s so nothing happens
• cursor leaves player bounds -> sets timeout 2s before going inactive

Requirements Checklist

• Feature implemented / Bug fixed
• Change has been verified in an actual browser (Chrome, Firefox)
• Tests cases passed
• Reviewed by Two Core Contributors

[welcome]

💖 Thanks for opening this pull request! 💖

Things that will help get your PR across the finish line:

• Run npm run lint -- --errors locally to catch formatting errors earlier.
• Include tests when adding/changing behavior.
• Include screenshots and animated GIFs whenever possible.

We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can.

[gkatsev]

Thanks for the PR!
I wonder if a better way of doing this would be to save the interval id and the handler and then clear the interval on mouseenter and then recreate it on mouseleave? Though, storing the previous inactivity timeout would be easiest.

[gkatsev]

if someone set a custom inactivity timeout, this will reset that value.

[gkatsev]

We could store the inactivityTimeout in this.cache_ and then reset to that value.

[xjoaoalvesx]

Good point, I now save the value in the cache object on mouseenter and retrieve it on mouseleave

[gkatsev]

we should store the control bar object in a variable (to also re-use below) and do a null-check on it.

Additionally, it would be better to call .on directly on the contrlBar itself as the handler will automatically be removed when the control bar is disposed. I don't think the current event handler gets cleared up automatically, currently.

[xjoaoalvesx]

Edit: Actually, I think I understand what you mean: Besides creating a local variable to reuse and test for null on the player, we should create separate methods on the controlBar class to add the handlers and call them from the player.

[gkatsev]

Basically, this.on(anotherComponent, 'mouseenter', () => {}) vs anotherComponent.on('mouseenter', () => {}). I am unsure that the first one will have the mouseenter event removed when the component is disposed where-as I'm certain the second one will be done properly.

Ultimately, the code should end up something like:

const controlBar = this.getChild('controlBar');

if (controlBar) {
  controlBar.on('mouseenter', (event) => /* ... */);
  controlBar.on('mouseleave', (event) => /* ... */);
}

[xjoaoalvesx]

I completely understand what you say so now we can see that on the code.

[welcome]

Congrats on merging your first pull request! 🎉🎉🎉