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

Add :blob-emotes: #526

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

Conversation

NoRePercussions
Copy link
Contributor

@NoRePercussions NoRePercussions commented Oct 6, 2023

:party-blob: demo

What?

Adds the following emotes to tracker:

  • :party-blob:
  • :blobsob:
  • :blobtoiletspin:
  • :blobsad:
  • :blob-spin:
  • :blob-go:
  • :blob_excited:
  • :blobeyes:
  • :blobfearful:

(Inconsistent) names are taken from Slack so you can copy-paste from it.

Why??

It's iconic.

Potential Problems

  • Essentially has to scan the whole DOM: I've made it as efficient as I know how to with JQuery, and it only runs after the whole page has rendered and the rest of our JS has loaded, but it's still not preferable.
  • Relies on external images: They could be replaced with self-hosted images, but that felt excessive. If an image fails to load, it will be replaced by the no-image icon and the original text.
  • Could mildly worsen line spacing: if used infrequently in large blocks of text.
  • Not useful: People seem to like it.
  • Visual clutter: yes

@wjiang42
Copy link
Contributor

wjiang42 commented Oct 6, 2023

  • idk JQuery :(
  • I think standard practice is to self-host for longevity, strongly prefer not having external assets
  • Personally I think this is fine
  • Yes
  • Yes (I do think reducing clutter on Tracker is important, will think about this point more)

@NoRePercussions
Copy link
Contributor Author

https://cmuabtech.slack.com/archives/CKLMNB5PX/p1696632545513259

The most recent commit definitely needs review... this site seems pretty JS-light so I can't figure out how you'd prefer it to be done.

This also helps with the clutter a bit, since like nicknames it won't be visible to non-members.

@DaAwesomeP
Copy link
Member

Yeah any and all assets need to be downloaded into the repo. Especially since Slack does not specifically host these asserts for others to use, those links are highly likely to break. In general things break if you rely on external resources; this was a major part of fixing abtech.org.

May I suggest a more performant/controllable alternative? Add a specific emote field to events that gets appended to the end of the event title when appropriate.

  • That way you don't have long strings like Scotty Saturday :blobtoiletspin: :blob-spin: :blob-spin: :blob-spin: :blob_excited: in say, a dropdown box (where you can't have images) to assign a quote/timecard to an event.
  • Also the emotes definitely can't appear in quotes/invoices.
  • In general it will be way more work to scrub the titles from every possible place they might be unwanted than to just add a field.
  • We can add an option to turn off the emotes for people who do not like the flickering images.
  • We wouldn't have to scan the DOM and can add the images server-side.

If we do stick with the scanning DOM method, then we still need to scrub the emotes server-side when they shouldn't appear.

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.

3 participants