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

Create vue sensor status page #42

Merged
merged 8 commits into from Jan 9, 2020
Merged

Create vue sensor status page #42

merged 8 commits into from Jan 9, 2020

Conversation

@angustrau
Copy link
Member

angustrau commented Jan 7, 2020

image

I'd love to get everyone's opinion on how readable the code is, and whether it is simple enough to extend in the future without a lot of experience in Vue.

@angustrau angustrau self-assigned this Jan 7, 2020
@angustrau

This comment has been minimized.

Copy link
Member Author

angustrau commented Jan 7, 2020

I decided to write a React version for comparison. Personally I prefer it
https://github.com/monash-human-power/dashboard/tree/react

@harsilspatel

This comment has been minimized.

Copy link
Member

harsilspatel commented Jan 7, 2020

make sure you're getting enough sleep Angus! :D

Copy link
Member

harsilspatel left a comment

Looks good to me, can't wait to checkout the new website, great work, now get some rest @angustrau :D

@harsilspatel

This comment has been minimized.

Copy link
Member

harsilspatel commented Jan 7, 2020

woah wtf, i just read the React version comment, that's incredible Angus, like seriously, too good! ^_^

Copy link
Member

hallgchris left a comment

Looks very cool, I think I could get my head around this! Haven't looked at the react properly yet.

Is the intention to keep frontend/backend separate or will they be merged together like they were before?

@angustrau

This comment has been minimized.

Copy link
Member Author

angustrau commented Jan 8, 2020

@hallgchris yep, they will be kept separate. The backend will be configured to serve the frontend's build output as static files. This will make it easier to refactor the backend in the future.

@khanguslee

This comment has been minimized.

Copy link
Member

khanguslee commented Jan 8, 2020

Good job as always! I may be biased (since I've worked with React a lot more), but looking at the Vue code kinda hurts my eyes compared to the code in your react branch. I just don't like how the html, js and css code are in the same file and I can see it get messy (expecially with inexperienced programmers)

Going forward, happy to merge this branch into the main vue branch, but I think moving towards React is the way to go. Expecially since how BOOSTERS will also be using React.

@maanika
maanika approved these changes Jan 9, 2020
@angustrau

This comment has been minimized.

Copy link
Member Author

angustrau commented Jan 9, 2020

I'm merging this branch, but going forward the rewrite will be in React, and be developed in the react branch instead.

@angustrau angustrau merged commit a9c08bf into vue Jan 9, 2020
1 check failed
1 check failed
Codacy/PR Quality Review Codacy was unable to analyse your pull request.
Details
@angustrau angustrau deleted the angustrau/vue-sensor-status branch Jan 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.