Skip to content

Conversation

@emvaized
Copy link

@emvaized emvaized commented Jul 22, 2023

This PR:

  • Removes check pointerEvents=none on navbar button, which resulted in script not working for me on FF 115
  • Changes default color of effect to browser's color of navbar button's color (--button-hover-bgcolor)
  • Adds new options:
    • filterDy, which don't process mouse movements greater than {gradientSize}px from top of the screen, in order to reduce system load (default: true)
    • cacheButtons, which stores all toolbar buttons on script init and then reuses the list on each draw iteration (default: false)
    • includeUrlBar, which applies effect to url bar as well (when it's not focused) (default: true)

Copy link
Owner

@aminomancer aminomancer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty neat, thanks. I have some feedback, mostly nitpicks.

if (el.disabled) {
return this.clearEffect(area);
}

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code below needs to be fixed, or there is a bug when you move the mouse around the top of the page on about:config or about:preferences, etc. It's because they are in the parent process so their true event targets pass all the way up to the chrome window listener (normally the event target we see would be <browser>).

It's better if we just use MousePosTracker.

      let x = MousePosTracker._x - this.getOffset(area).left - window.scrollX;
      let y = MousePosTracker._y - this.getOffset(area).top - window.scrollY;

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I didn't know about this bug, so my first thought was this check was added just for UX convenience. So now this should be handled by new getComputedStyle(el).pointerEvents == "none" check, right?

@emvaized emvaized requested a review from aminomancer July 23, 2023 22:46
Copy link
Owner

@aminomancer aminomancer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good, there is still some minor cleanup to do though

@emvaized emvaized requested a review from aminomancer July 25, 2023 23:24
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