Legend-Master / discord-better-image-viewer

A Better Discord plugin for better image viewing experience
https://betterdiscord.app/plugin/BetterImageViewer
MIT License
6 stars 0 forks source link

Reason for decline #3

Closed zerebos closed 8 months ago

zerebos commented 11 months ago

It seems you have your DMs turned off to the bot which is where the reason normally goes.

Plugin has been declined with the following reason:

This performs a document.querySelector on every single mutation observer event blindly and without filtering. This can have a major performance effect for users with lesser PCs. Please use filterings of the mutation records provided in the observer function to limit the scope or prevent the need for a querySelector at all. If you're unsure how to do this please reach out to myself (Zerebos) or to the community in the Programming channel in the BetterDiscord server.

Legend-Master commented 11 months ago

I made some changes in fe205fe7036f81b2171efb5b98c8788df54cbd6a to only do querySelector on an added node if it has layer_ad604d class (which is a random parent node and may change in the future)

I didn't do this at the beginning is because I'm not sure if it's gonna break frequently because of the narrowed match on parent node added, may I ask is there a better way to detect the image view?

zerebos commented 8 months ago

It's much better now in the current version, though you can use some more resilient selectors. Additionally, it seems that it does not work on image galleries. If you want some help, feel free to reach out in the programming channel on the BD Discord server.

Legend-Master commented 8 months ago

@rauenzi

Additionally, it seems that it does not work on image galleries.

I'm aware of it not working on gallery view #1, but I don't have time for it right now

though you can use some more resilient selectors.

What would that be? Is there a better way other than using tag + class names?

zerebos commented 8 months ago

Instead of doing for example node.matches('div.layer_ad604d') you can do node.matches('div[class*="layer_"]') or instead of node.querySelector('.loadingOverlay__4d818') do node.querySelector('[class*="loadingOverlay__"]')

zerebos commented 8 months ago

BTW to clarify the resilient selector stuff is not preventing your plugin from being accepted. I'm just debating about #1 just because I can see users stumbling into that very quickly and coming to bd support.

Legend-Master commented 8 months ago

Instead of doing for example node.matches('div.layerad604d') you can do node.matches('div[class*="layer"]')

Got it

I'm just debating about https://github.com/Legend-Master/discord-better-image-viewer/issues/1 just because I can see users stumbling into that very quickly and coming to bd support.

I'll take a look when I got the time, and submit again after implementing it then

Legend-Master commented 8 months ago

@rauenzi I just added gallery view support (6c995bccaf2b0dd138c2fd8995a071b9bef4e9ff) and changed selectors to '[class*="loadingOverlay__"]' style (b6286fe6de431c5543fcd5ebb28284d125a03947), would mind take a look again?

zerebos commented 8 months ago

Yeah it looks good, resubmit it to the site and I should be able to approve it today!

zerebos commented 8 months ago

btw can you join the BetterDiscord server? I don't currently see you in there.