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

post REST & SPA comments #25

Open
Dieterbe opened this issue Oct 8, 2014 · 6 comments
Open

post REST & SPA comments #25

Dieterbe opened this issue Oct 8, 2014 · 6 comments

Comments

@Dieterbe
Copy link
Contributor

Dieterbe commented Oct 8, 2014

#21 brought a revamped admin UI as a single page app, based on a rest api exposed by the go program. neat stuff, and it got merged.

however there's a few small remarks I'ld like to addressed:

  • when no fields are filled in, and you click "add route", it doesn't show error messages like the previous codebase did.
  • when a route comes back up, it stays red, until you reload the page. might be nice to automatically update the online state. (if it's not too much of a hassle)
  • removeRoute needs a comment explaining why we create a new map[string]string

cc @pwielgolaski

@pwielgolaski
Copy link
Contributor

when no fields are filled in, and you click "add route", it doesn't show error messages like the previous codebase did.

I did it in a way that you can not press Add if fields are invalid, do you think it would be better to click it and get some error message?

when a route comes back up, it stays red, until you reload the page. might be nice to automatically update the online state. (if it's not too much of a hassle)

I think that I can do autorefresh every minutes if you think it would be what you want

removeRoute needs a comment explaining why we create a new map[string]string

I can add comment, the reason is to fulfil contract with ServeHTTP method

@Dieterbe
Copy link
Contributor Author

I did it in a way that you can not press Add if fields are invalid, do you think it would be better to click it and get some error message?

yeah, I think it's better to be more clear that there's an error in the input

I think that I can do autorefresh every minutes if you think it would be what you want

let's not do that. this could become very annoying if it refreshes right when you're in the middle of something.
i was hoping maybe there was an elegant, simple way to update on the fly (by polling for updates in the background every second or so, or maybe even the server pushing updates when the route state changes)
further down the road, i really want the ui to display performance metrics in real-time and have them auto-update (without refreshing the page)

@pwielgolaski
Copy link
Contributor

I think that I wasn't clear enough, when I said refresh it is not refresh of whole page, but only list of routes.

You can grab gist version of app.js and give a try: https://gist.github.com/pwielgolaski/520b133e72e98dc512fa

@Dieterbe
Copy link
Contributor Author

aha yes that updates the routes status nicely. is there any possibly race conditions when you're trying to edit a route and save it but at the bad time it updates with fresh values from the refresh, or something?
otherwise, let's merge this? 👍

@pwielgolaski
Copy link
Contributor

I dont see any issue with race condition, but I'd like to wait for #22 to be fixed, as it is annoying that you can get routes in different order and it will automatically refresh list changing order of routes.

@Dieterbe
Copy link
Contributor Author

hey @pwielgolaski any interest in taking back up the web interface? you did nice work last year but i had to rearchitect some core code and concepts. now that stuff has matured a bit more and it's mostly the web and telnet interfaces that need some love :)

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

No branches or pull requests

2 participants