search

greasyfork-org / greasyfork

An online repository of user scripts.
https://greasyfork.org
GNU General Public License v3.0
1.42k stars 426 forks source link

Make StylusDetection Faster #1165

Closed cyfung1031 closed 9 months ago

cyfung1031 commented 11 months ago

The 200ms shall not be necessary since the Promise can be directly resolved by onMessage

cyfung1031 commented 9 months ago

Thanks for the suggestion. The general idea is good, but see the comments.

If you're not interested in adjusting the PR, let me know (or just don't say anything for a bit) and I'll do it myself, but not sure if you'll get credit in git history if I do it that way.

Thanks for the review. It's almost 2 months ago I forgot this PR too. Understand your points. Let me check later and get back you :)

cyfung1031 commented 9 months ago

@JasonBarnabe I have changed the code as per your request. The event listener is now scoped inside canInstallUserCSS() and to be removed after timeout or detection.

localStorage's stylusDetected can be also removed if you want.

JasonBarnabe commented 9 months ago

I deployed this and tried it out and it didn't work. Seems like the message listener never fired for the style-version response from Stylus. Not sure why. I backed it out.

JasonBarnabe commented 9 months ago

I believe it may be because Stylus will only respond once and there's another query happening in managers.js.

The previous code added the message listener immediately whereas yours only does so when the button is pressed, but by that time, Stylus has already fired its one and only message.

cyfung1031 commented 9 months ago

Noted. Let me check it again later :)