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

Frontend rewrite using Angular #87

Merged
merged 90 commits into from
Jul 8, 2021
Merged

Frontend rewrite using Angular #87

merged 90 commits into from
Jul 8, 2021

Conversation

hrueger
Copy link
Collaborator

@hrueger hrueger commented May 22, 2021

Advantages:

  • HTML elements are not created in strings and then added to the DOM, instead Angular handles all of that based on the template and its logic
  • Removed the call to the external QR Code Server as it is now generated in the frontend
  • No code duplication (e.g. navbar)
  • improved credentials form
  • Listener Clients in Producer View are sorted based on their Active / Inactive Status
  • Complete typings for all Objects
  • Improved UI with different colored buttons and icons
  • Loading Spinners
  • Progressive Web App functionality (e.g. to Home Screen)
  • better confirm dialogs
  • Chat History is kept until page refresh

Current Status:

  • Base structure
  • Home Screen
  • Tally Screen
  • Producer Screen
  • Settings Screen
  • Cloud Functionality
  • Icons
  • Fix no device states on initial load (producer screen)
  • Move sockets into service
  • SweetAlert
  • Loading indicators
  • Separate About Page
  • Redesign Home Page
  • Ports in use
  • Validation
  • Authentication
  • Chat
  • NoSleep
  • Favicon
  • PWA Stuff
  • Build Process
    • CLI
    • Electron
  • GitHub Actions

fixes #86

@hrueger
Copy link
Collaborator Author

hrueger commented May 22, 2021

@josephdadams I have thought about the release process. If I read the README.md correctly, currently it can only be installed by cloning or downloading the GitHub Repository, installing the dependencies and then running node index.js.

Here are my ideas (feel free to add some or criticize mine ;-) )

  • Docker Image: It could be run in the cloud and on every machine which has Docker installed with docker run tallyarbiter -p 4455:4455.
  • NPM Package: Could be run with npm i -g tallyarbiter and then tallyarbiter
  • Electron + Installer: With Electron we could make a full-featured cross-platform desktop app. Users could download the installer and the run TallyArbiter like and other software. Could be great for users with less terminal experience. However, I don't know if this works on a Raspberry Pi with the lite image (so without the GUI)

What do you think? With GitHub Actions it would not be a problem to create a GitHub Release automatically on every git tag. We could go with all the release options to provide maximum flexibility for users.

@josephdadams
Copy link
Owner

@hrueger great thoughts! The electron package is definitely something I’ve wanted to do but I’ve been holding off until the codebase was more manageable. I even prototyped it and had it working fine but didn’t want to release it until I had done something to compartmentalize the areas of code.

I’m fine with the other methods as well.

You’re correct that electron won’t run on Pi’s without desktops, and pi compatibility is important to me. Bitfocus Companion achieves this through a headless script, so perhaps we could model after that.

@hrueger
Copy link
Collaborator Author

hrueger commented May 22, 2021

You’re correct that electron won’t run on Pi’s without desktops, and pi compatibility is important to me. Bitfocus Companion achieves this through a headless script, so perhaps we could model after that.

Could Pi uses just use the tallyarbiter npm package instead of the electron version?

@hrueger
Copy link
Collaborator Author

hrueger commented May 22, 2021

@josephdadams Is there a reason why the navigator.vibrate method is called like this:

let successBool = window.navigator.vibrate(100, 30, 100, 30, 100);

The TS Autocompletion wants

let successBool = window.navigator.vibrate([100, 30, 100, 30, 100]);

Are there devices which don't want an Array here?

@josephdadams
Copy link
Owner

You’re correct that electron won’t run on Pi’s without desktops, and pi compatibility is important to me. Bitfocus Companion achieves this through a headless script, so perhaps we could model after that.

Could Pi uses just use the tallyarbiter npm package instead of the electron version?

I believe so.

@josephdadams
Copy link
Owner

@josephdadams Is there a reason why the navigator.vibrate method is called like this:

let successBool = window.navigator.vibrate(100, 30, 100, 30, 100);

The TS Autocompletion wants

let successBool = window.navigator.vibrate([100, 30, 100, 30, 100]);

Are there devices which don't want an Array here?

Tbh I probably copied that from somewhere else that way. So whatever works.

@hrueger
Copy link
Collaborator Author

hrueger commented May 22, 2021

Is there already some logic in place to identify web clients? If I refresh my Tally Page (e.g. with F5), a new listener appears in the producer / settings view and the old one gets greyed out.

I'd create a unique ID on the client side and save this in localStorage. Then, I could send this to the socket server which could identify based on that id instead of the socket id. What do you think?

This is probably needed in order to save settings for each client (like in #11).

@JTF4
Copy link
Collaborator

JTF4 commented May 22, 2021

I think that would require a recode of how all of the clients handle their ID. Is there any way to make it so that clients that support local storage use your method and older versions use the current system?

I for one would be all for a system that can rematch disconnected clients with their last assigned source though.

@josephdadams
Copy link
Owner

My thinking had been:

  1. Listener connects
  2. Server generates id and sends to client
  3. Client saves this id and sends it on subsequent requests
  4. If the id is not valid for some reason, it is just treated like a new connection and a new id is made.

We can use this same id on the server side to send preferences like preferred device id, colors, etc. This way the server can relate the id to the type and enabled features

@hrueger
Copy link
Collaborator Author

hrueger commented May 22, 2021

Sounds good. As this also requires changes in the backend, I'll create a separare PR for that once this one is done.

@josephdadams
Copy link
Owner

Great! I had started converting all the source types to their own classes but not in ts. I’ll push that to its own branch in case it is helpful. My thinking was to try to abstract it as much as possible so all the source specific logic was contained within that file.

@hrueger
Copy link
Collaborator Author

hrueger commented May 23, 2021

Thanks @josephdadams I'll take a look at that.
Another question: Do you mind if I move the version (at the bottom of the settings page) and the license (at the welcome page) to a separate about page? This way, I could redesign the welcome page (for example with some big buttons to go to the tally / producer / settings page. Or do you prefer having the license directly visible on the welcome page?
We could also go with For licenense information go to the about page.

@josephdadams
Copy link
Owner

It’s fine with me to move those to another page. I’m not particular about that at all. Thanks!

@josephdadams
Copy link
Owner

Test went great. I’ll be looking to merge this week. Anything major left before I do so?

@hrueger
Copy link
Collaborator Author

hrueger commented Jul 5, 2021

Great 👍 I don't think that there's anything left. Once it's merged you just need to enable GitHub Pages for the gh-pages branch if you haven't done that yet.

@josephdadams
Copy link
Owner

@hrueger will the gh-pages branch automatically come over with this PR, or do we need a separate PR for that?

@hrueger
Copy link
Collaborator Author

hrueger commented Jul 7, 2021

It should be created automatically.

@josephdadams josephdadams merged commit df47945 into josephdadams:master Jul 8, 2021
@josephdadams
Copy link
Owner

@hrueger I've merged the PR. Fantastic job. Thanks so much!

It looks like the actions are running and the builds are being created. I've created a new tag for release which fires off the actions, but not seeing the build files showing up with the release. What am I missing?

@hrueger
Copy link
Collaborator Author

hrueger commented Jul 8, 2021

@josephdadams Great news 😀

What am I missing?

I always create the tag using the command line with git tag v2.0.0 and then push it with git push --tags. This triggers the action which automatically creates a draft release. The artifacts are then added to the draft release. Once that has finished, I add my changelog texts to the Release and publish it.

The upload process is handled entirely by electron-builder, so I don't know if it can add artifacts to already published releases...
I assume that you created the release which auto-created the tag.

You could try creating the tag v2.0.0 using the command line. I generally use the same tag version as the npm version, so that would be v2.0.0 instead of v2.0.

@josephdadams
Copy link
Owner

I created the release through the web interface.

I took at look at the build.yml, it looks like all the "upload" parts are commented out. Is that correct?

@josephdadams
Copy link
Owner

Ok, I clearly just did something wrong, because gh-actions had a draft release of 2.0.0 for me. I released that and it had the binaries attached as expected. Sorry for my naivety, I've not done anything with actions up to the point. Thanks!

@hrueger
Copy link
Collaborator Author

hrueger commented Jul 8, 2021

No problem, glad it works now 👍

@hrueger
Copy link
Collaborator Author

hrueger commented Jul 8, 2021

I took at look at the build.yml, it looks like all the "upload" parts are commented out. Is that correct?

This upload adds build artifacts directly to the workflow run. I found that quite useful for testing but I don't think this is needed in production.

@MatteoGheza
Copy link
Collaborator

@hrueger since GH releases currently work, can you implement Electron autoUpdater?

@hrueger
Copy link
Collaborator Author

hrueger commented Jul 8, 2021

Sure 👍

@mattv8
Copy link
Contributor

mattv8 commented Jul 8, 2021

Had a chance to look at the Angular frontend, looks fantastic. Amazing work @hrueger and @josephdadams!! 👐👐

@mattv8
Copy link
Contributor

mattv8 commented Jul 8, 2021

Just as an FYI: after some more testing trying to get the Electron app running on a Windows environment (building the TA and Desktop UI from source) I get the error generated by Electron:
`

elfy@Matt-Surface:/mnt/c/Users/mattv/Downloads/Git Repositories/TallyArbiter$ npm run desktop`

[email protected] desktop
electron .

[16432:0708/133256.634715:FATAL:setuid_sandbox_host.cc(158)] The SUID sandbox helper binary was found, but is not configured correctly. Rather than run without sandboxing I'm aborting now. You need to make sure that /mnt/c/Users/mattv/Downloads/Git Repositories/TallyArbiter/node_modules/electron/dist/chrome-sandbox is owned by root and has mode 4755.
`

Windows Subsystem for Linux has no root user to chmod the directory chrome-sandbox to, so unfortunately the build stops there. Are there plans for publishing pre-built releases for the Electron app in the future for us Windows laypeople?

Also to add a note for Linux people, the following dependencies will be needed to build with Electron, and can be installed with the following command:
sudo apt install libappindicator3-1 libasound2 libatk-bridge2.0-0 libatk1.0-0 libc6 libcairo2 libcups2 libdbus-1-3 libexpat1 libfontconfig1 libgbm1 libgcc1 libglib2.0-0 libgtk-3-0 libnspr4 libnss3 libpango-1.0-0 libpangocairo-1.0-0 libstdc++6 libx11-6 libx11-xcb1 libxcb1 libxcomposite1 libxcursor1 libxdamage1 libxext6 libxfixes3 libxi6 libxrandr2 libxrender1 libxss1 libxtst6 lsb-release xdg-utils

@MatteoGheza
Copy link
Collaborator

Have you tried removing node_modules and re-running npm install in UI?

@mattv8
Copy link
Contributor

mattv8 commented Jul 8, 2021

Just tried it, here's what I see:

root@Matt-Surface:/mnt/c/Users/mattv/Downloads/Git Repositories/TallyArbiter# rm -rf node_modules
root@Matt-Surface:/mnt/c/Users/mattv/Downloads/Git Repositories/TallyArbiter# cd UI
root@Matt-Surface:/mnt/c/Users/mattv/Downloads/Git Repositories/TallyArbiter/UI# npm install
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE package: '@angular-devkit/[email protected]',
npm WARN EBADENGINE required: {
npm WARN EBADENGINE node: '^12.14.1 || ^14.0.0',
npm WARN EBADENGINE npm: '^6.11.0 || ^7.5.6',
npm WARN EBADENGINE yarn: '>= 1.13.0'
npm WARN EBADENGINE },
.
.
.

up to date, audited 1342 packages in 5s

11 vulnerabilities (6 moderate, 5 high)

To address issues that do not require attention, run:
npm audit fix

Some issues need review, and may require choosing
a different dependency.

Run npm audit for details.

root@Matt-Surface:/mnt/c/Users/mattv/Downloads/Git Repositories/TallyArbiter/UI# cd ../
root@Matt-Surface:/mnt/c/Users/mattv/Downloads/Git Repositories/TallyArbiter# npm run desktop

[email protected] desktop
electron .

sh: 1: electron: not found
root@Matt-Surface:/mnt/c/Users/mattv/Downloads/Git Repositories/TallyArbiter#

@MatteoGheza
Copy link
Collaborator

MatteoGheza commented Jul 8, 2021

Try removing node_modules in the root directory of the project and in UI and then run npm install in the root directory, and then try again.
If it doesn't work ("electron not found"), try running npm i -g electron

@hrueger
Copy link
Collaborator Author

hrueger commented Jul 8, 2021

@mattv8 Just that I understand what you are trying to accomplish: You are inside Ubuntu on WSL2 and are trying to run the Electron version from inside?

And there is a Windows Installer available for download on the Releases page. Or are you looking for a portable version?

@mattv8
Copy link
Contributor

mattv8 commented Jul 8, 2021

@hrueger that's exactly right, Ubuntu 18.0.4 WSL2. I'm just trying to understand how to build the Electron side of the project for myself.

Also, I'm a goober and didn't notice the releases page here: https://github.com/josephdadams/TallyArbiter/releases Typically on a repo you see the releases from the project root (https://github.com/josephdadams/TallyArbiter) on the right column so I had just missed that Joseph created a Releases page.

@MatteoGheza Tried as you suggested, and saw the same error as before:

[17352:0708/141908.110016:FATAL:setuid_sandbox_host.cc(158)] The SUID sandbox helper binary was found, but is not configured correctly. Rather than run without sandboxing I'm aborting now. You need to make sure that /usr/lib/node_modules/electron/dist/chrome-sandbox is owned by root and has mode 4755.
/usr/lib/node_modules/electron/dist/electron exited with signal SIGTRAP

There's a mention of this error on Electron's repo that is marked as solved, so this must be a limitation of WSL to build Electron: electron/electron#17972

@hrueger
Copy link
Collaborator Author

hrueger commented Jul 8, 2021

I have not yet managed to run gui apps and therefore electron apps inside WSL2. I've read that support for those is coming with one of the next Windows Updates.
So I don't know if you can run it inside Ubuntu in WSL2.

But you could try building and running on Windows?

@mattv8
Copy link
Contributor

mattv8 commented Jul 8, 2021

I will have to figure out how to build and run on Windows, as I'm unfamiliar with NodeJS on anything other than Linux & derivatives. I'll give it a try and report back.

@hrueger
Copy link
Collaborator Author

hrueger commented Jul 8, 2021

Did you try --no-sandbox?

By the way: running electron . is not actually building the app, but instead just running the electron binary with the path to the package directory as the first argument. The only building we have is the build of the Angular Frontend and the build of the installers.

@hrueger
Copy link
Collaborator Author

hrueger commented Jul 8, 2021

@josephdadams can you enable the "Releases" tab in the repo settings somewhere? I wonder why it does not appear.

@hrueger
Copy link
Collaborator Author

hrueger commented Jul 8, 2021

I will have to figure out how to build and run on Windows, as I'm unfamiliar with NodeJS on anything other than Linux & derivatives. I'll give it a try and report back.

Good luck, it should actually not be much different. You just need to make sure that you run npm i from the Windows env again (after you have deleted the node_modules folder as then the electron installer script will download the correct binaries. Otherwise you still have Linux binaries under Windows and I don't think that this works well 😉

@josephdadams
Copy link
Owner

@hrueger I added you to the repo to see if you can find it because I could not :)

@hrueger
Copy link
Collaborator Author

hrueger commented Jul 8, 2021

Thanks. I don't have permissions to do that, but I think this is the button you need to click on:
Screenshot_20210708-223728_Firefox.jpg

In that modal you should be able to avtivate the releases tab.

Sorry for the bad quality, but I'm just on my phone.

@josephdadams
Copy link
Owner

I figured it out with your help :)

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.

Frontend rewrite using Angular
5 participants