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

Blinkmoji #11

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Blinkmoji #11

wants to merge 8 commits into from

Conversation

mbalasz
Copy link

@mbalasz mbalasz commented Nov 1, 2021

Hi Fede,

As discussed I gave it a shot to integrate the Blinkmoji web app with your repo.

Summary of changes

  • I added subpages to your main URL. So the starting point for blinkmoji should be https://fheinz.github.io/Blinkenlights/WebApp/blinkmoji
  • I did not change anything in your main page
  • I refactored your script.js to extract common functionality for just controlling the blinkenlights to a separate script called blinken.js
  • The Blinkmoji app is basically a blinkenlights emoji chat room. I added a functionality of joining arbitrary rooms and people in one room can send blinkenlight emojis to each other.
  • The app is still very crude and I only did basic local testing. I definitely encourage to test the app with multiple blinkenlights.

Requirements on the server side
As specified on my original repo page (https://github.com/mbalasz/blinkmoji#requirements) the server hosting the web app needs to have ImageMagick installed. It also needs the Noto Color Emoji font.

Local testing

  1. npm install to download some extra node modules
  2. In order for browserify to work I needed to do a small change in package.json of the module that provides emoji icons. So locally I recommend you add those lines
  3. node server.js
  4. The web app should now be working under localhost:3000/blinkmoji

New Branch?
I'd be happy to merge this into a separate branch as opposed to main to start with. However, I think you'd need to first create such branch.

I'm happy to chat more about this PR before moving forward. Just wanted to have some draft ready.

socket.on("chat message", (msg) => {
console.log("Received message: " + msg);
const user = findUser(socket.id);
printEmoji(String.fromCodePoint(msg.codePointAt(0)), user.room);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found a bug :)

user can be undefined. Leaving a comment for myself to fix it asap.

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