search

ericwbailey / millennials-to-snake-people

🐍 Chrome extension that replaces occurrences of 'Millennials' with 'Snake People'
https://chrome.google.com/webstore/detail/millennials-to-snake-peop/jhkibealmjkbkafogihpeidfcgnigmlf
Do What The F*ck You Want To Public License
194 stars 115 forks source link

Modify the walk function to use document.createTreeWalker #7

Closed samford closed 9 years ago

samford commented 9 years ago

I reworked the walk() function to use document.createTreeWalker() (rather than the custom recursive function) and it's just a hair faster than before. Using createTreeWalker() we'll benefit from any related improvements that are made in the future, at least.

ericwbailey commented 9 years ago

Hey there, haven't forgotten about this. Hoping to review this and the other two PRs over the weekend. And sorry about the delay!

samford commented 9 years ago

No worries! Just as a heads up, I spent some time a couple weeks ago creating an Options page for the extension and making the necessary modifications so that users can toggle each replacement off or on (rather than it being all or nothing). I haven't committed it to my fork yet but once these other PRs are merged I'll wrap it up, push it, and create a PR to discuss it.

ericwbailey commented 9 years ago

Sorry again it took so long, it's been a crazy summer and I wanted to take some time to figure out how document.createTreeWalker() worked (my JS skills aren't so hot).

The iFrame and observers are fantastic additions, but to be honest, I kind of enjoy that it's an all-or-nothing affair that sits quietly in the background. I figure once a user has opted in, Incognito mode or an alternate browser is a viable workaround - an Options panel seems kind of like overkill.

samford commented 9 years ago

It's been a very busy summer for me as well, so it's all good. Thanks for accepting all these pull requests.

I can totally understand wanting to keep it simple. More than anything, I was curious to see if I could create an options page like I imagined (and how I would go about doing that), so I'm satisfied to write it off as an exercise and keep the extension how it is. I might still push the options version to a branch of my fork (sometime in the near future) just in case you or anyone else wants to fiddle with it but I won't create a PR.

ericwbailey commented 9 years ago

Yeah, I felt bad because you'd definitely put a lot of work in. If you do push the option up to your fork, let me know and I'll add a link to your repo in the Readme for people that would like the option!