search

ezaquarii / vpn-at-home

1-click, self-hosted deployment of OpenVPN with DNS ad blocking sinkhole
GNU General Public License v3.0
1.11k stars 90 forks source link

[frontend] Auto-scroll output in server deployment dialog #36

Closed arpreq closed 5 years ago

arpreq commented 5 years ago

Fixes #38, closes #26

ezaquarii commented 5 years ago

what I did with the triple dots feels more like a hack than the correct way to handle receiving a single string vs an array of strings.

Output is received in an array. Always.

It works, but is there a better/more preferred way to do this?

It's ok. Coding-in any exceptional paths will only hurt maintainability.

arpreq commented 5 years ago

Output is received in an array. Always.

From what the deployment script returns, yes, but this is not the case for line 69 in onFinish(): this.pushAutoScroll('Deployment finished.');

Please see my comment here https://github.com/ezaquarii/vpn-at-home/pull/36#discussion_r244611367

I've made the other requested changes but haven't made any new commits yet because they're so small.

ezaquarii commented 5 years ago

I think you accidentally closed discussion on other review points. I re-opened them. Let's address them and make it landed. It's close to production quality now.

arpreq commented 5 years ago

I closed them because I had already resolved them on my end (just without a commit).

I probably should have added comments to each saying that I've taken care of them, sorry.

Anyway, I'll go ahead and make what is hopefully the final commit :)

ezaquarii commented 5 years ago

Ok, many thanks for the contribution. I'm going to give it a final look tomorrow. I wish you a Happy 2019 🎉.

arpreq commented 5 years ago

Thank you for your patience! And thanks for leaving this issue for a newbie, even though it turned out to be a bit more complicated than it looked.

Happy new year to you as well!

PS: I found a small issue in deployment.js that prevents the onStart() dialog from appearing. Going to submit an issue and another pull request if I get to it later today.