Speeds up the emoji search by 93%. #215
No reviewers
Labels
No Milestone
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: rDrama/rDrama#215
Loading…
Reference in New Issue
There is no content yet.
Delete Branch "<deleted>:emoji-search-fix"
Deleting a branch is permanent. Although the deleted branch may exist for a short time before cleaning up, in most cases it CANNOT be undone. Continue?
Improvements:
Should be nearly identical client-side.
unreadable diff, too many pointless changes to review
I accidentally used autoformat on one file. Other than that -- ignore the diff changes on emoji_modal.js, I deleted the file and started over. It's really not that much tbh. Added a special attribute, data-emojis, on the html of the elements which u can type emojis in, which accounts for some other changes
revert all unnecessary changes so i can actually review this
Speeds up the emoji search by 93%.to Speeds up the emoji search by 93%.Note that I have not actually run the code yet. That is pending me more having more time. Also I know fuck all about javascript. I mean technically I worked with it like five years ago but that was back when orangeman was in office. So I will leave reviewing the main logic to someone that knows JS. Maybe Doctor Transmisia or Nekobit, the original authors.
If possible, @transbitch, could you briefly summarize the advantages/disadvantages of your approach/Dr T's approach, from a technical perspective? Like, why did you need to rewrite emoji_modal? Would it be possible to make changes to this file that weren't as invasive, or is a rewrite of the caliber that you are engaging in required?
I ask because this file has been modified quite a bit by @Aevann, probably in response to bugs he encountered in managing the site, which you may not be privy to. Therefore, a major rewrite could undo the work that @Aevann has put into the file and degrade user experience.
Not strictly necessary, but the thing called "speed-emoji-modal" IS NOT A MODAL, so the original class name is a misnomer. It's a dropdown, and "quick" makes more sense to me than "speed", so while we're at it might as well change it to "quick emoji dropdown". If it really ruffles some feathers then I can change it back.
OK
It conflicted with stuff while I was writing, so I put it in its own scope to resolve the error. It doesn't conflict anymore, but putting it in its own scope is good practice, so I left it in. I can revert this if required. This file needs some work anyways; using
top
andleft
can cause the browser to unnecessarily reflow the page. It should usetransform
, which enables GPU acceleration, and should maybe use a spritesheet/data URIs.Wdym the original MIT clause? Legally speaking, no licenses "changed," the old code still has its GNU Affero license and with the new code I wrote (not derived from the old code), I opted to license it under MIT. GNU Affero is a copyleft license, meaning that any derivative work needs to have the GPL license. It is legally a PITA. MIT has no such restriction, it is essentially a "do whatever" license. You can create a derivative project from MIT code and license it as GNU Affero, but you can't create a derivative project from GNU Affero and license it under MIT. So in other words, MIT is always compatible with GNU Affero, but Affero isn't always compatible with MIT. If you would like, I can offer both MIT and GNU Affero licenses, but again this is kind of pointless since MIT is compatible with everything. However, I understand why you would want to do this for conformity purposes in the codebase.
If there's one thing I love, it's conformity. ⬜
anyways, these cleanup changes that you are proposing may be better suited for a different PR without logic, so the scope of the changes is smaller
too many pointless changes, unreviewable
Pull request closed