Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: use prototypes and event dispatchers for updating email input values #229

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

old4ever
Copy link

General Information

What is this PR about?

This is my attempt at implementing a bit more robust method for input updates.
More specifically, it uses the input event dispatchers and prototypes to handle the intricacies of how React works.

Why did make it?

While using the extension, I have noticed that on some websites, email generation and autofill on inputs would generate an alias, but with any further interaction with the input, the alias would disappear. I immediately knew that the culprit was the JS framework React, specifically the virtual DOM. I decided to fix it myself, since most of the modern web runs some kind of React derivative. This would make using the button pointless, as you would need to click on the extension in your address bar anyway to copy the alias.
After checking the repo for similar reports, I was surprised to find that there was no mention of this issue, so I have decided to fix it myself and contribute back to make the extension more usable for everyone.

Demonstration of an issue (open to see the gif):

sl-ext-unpatched-unoptimized

How does it work?

React uses a synthetic event system and attaches its own getters and setters to DOM elements to manage their state and updates. Simply changing the value property of an input element doesn't inform React of the change because React doesn't listen to native DOM events directly.
While one solution would be to somehow use React's internal setters, this approach would be cumbersome to implement correctly and may vary from version to version of the framework.
A cleaner and more universal solution is to use the original setter of an input element prototype and then dispatch an input event. This would trigger React's event system and let it pick up the updated value in its virtual DOM.

Does that mean we need to have special handling for React?

Not really! The good thing about this method is that it's pretty much universal, as every functional website that's running JavaScript would have its setters on the element prototypes. This means that we don't need to check if the website is running React or not, since the prototypes are a core part of DOM APIs. In some rare edge cases where the prototype is not available, we can still fall back to just updating the value property and hoping for the best.

But wouldn't that add more overhead to maintaining the code?

Technically speaking, yes, but I have abstracted the input update mechanism behind a function, so it just takes an element and a new value. I have also added a bunch of inline comments to provide context about what each part of the function does and why. From a performance standpoint, we're basically already doing what straightforward value updating would do but breaking it down into steps.

Testing

Build:

  • Production build is successful
  • Linting is successful

Tested on:

  • Firefox v133.0.3 — working as expected
  • Google Chrome v132.0 — working as expected

Verified that:

  • New input update mechanism works as expected on:
    • Next.js (React-based) website - Link
    • Angular website - Link
    • Vue.js website - Link
    • Framework-less website - Link

Fix in action (open to see the gif):

sl-ext-patched-unoptimized

P.S.

I have also taken the liberty of dispatching a few additional events, mainly to mimic the actions a user would take to improve UX. For example, we will dispatch a change event alongside input, just in case some websites use a different listener. Additionally, dispatching focus/blur events should hopefully trigger some QoL features, like input validation and email availability. I did encounter a couple of annoyances while using the extension myself and testing this PR, such as not validating the email until you click the input again to bring it in/out of focus, only to receive a message that the service is not accepting the alias's domain.

P.P.S.

I hope it wasn't very painful to read this wall of text! 😅

I was initially thinking of creating an issue and putting these explanations there and just linking the PR to it. But it seemed a bit pointless since issues are meant to discuss the problem, and I have already fixed it myself.
Contributing to OSS is still new for me, and while I have tried to research as much as possible about the development style of this repo, I hope I didn't make any terrible mistakes. If I did, please let me know so that we can discuss them, and I will try to fix them! I'm eager to learn and open to any feedback or suggestions.

Thank you for considering this pull request!

more on why is in the inline comments.
tldr; for intended UX, i.e. input validation
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.

1 participant