-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Available upgrade notification #4906
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
Conversation
|
Thanks for the contribution! One thing though, I think the hit to the github endpoint should happen in the backend API. This avoids any CSP problems someone might have by putting the admin ui behind a proxy and this would take advantage of any Proxy server someone might be using in the backend. |
For sure, would this make the trick? |
This comment was marked as outdated.
This comment was marked as outdated.
7heMech
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
improvements:
- edit endpoint to only return latest version from github.
- on the frontend use existing version from healthcheck to compare.
- Keep tabs instead of spaces, in order to align to the rest of the repo.
Please STOP editing the same comment over and over without any changes of substance. It's a bit excessive, I got your point. I don't necessarily agree, but I got it and any feedback is valuable. On your last point, I use tabs. The file was previously using spaces and the "error" was introduced by my editor automatically being a de-alignment is a bit much here but sure I will fix it if it is required, just give me some time. |
Hey, it was a comment, now it's a review, I've edited it once I think, also it's unnecessarily fetching data twice. |
It was 7 edits and 1 review and now you are editing this too 😂 its honestly fine but I get worried by github emails thinking it's my day job. |
Eh, interesting, I don't get emails from edits, you might need to tweak your settings, but I might've edited it a lot hahah 😅 |
|
Try running a popular project. I have 3516 unread emails from github. |
|
Hahaha, no thanks! |
|
tabs... |
Dude, I am not smart enough to do it. The embedded on github visual studio does not see the difference. Jetbrain's IDEs and github desktop don't see the difference. And when I force it through notepad++ github sees the same space size as double even though it isn't so it again marks the lines as changed. If it is worth it to you submit a PR on my fork and I will merge it so it will show up here. And also tell me how you did it. |
|
ok one sec (I did it) |
|
I recommend using VS Code, especially for Node.js. But all IDEs support showing whitespace and converting from spaces to tabs. A quick Google will solve that for you. I do find it lacklustre that github's diff doesn't show whitespace properly though. |
They do but github diff doesn't detect the difference :( Anyway, do you think this is mergeable? |
I did make you a PR and github does see diff. |
|
Ah, I see what you meant. |
|
Yeah I'll merge it. I've got more feeback but it's easier if I make the changes myself before release. |
I am asking more in the sense of whether you need me to do anything. No pressure. Thank you for your great work, and for being patient with me ;) |
|
Docker Image for build 13 is available on DockerHub: Note Ensure you backup your NPM instance before testing this image! Especially if there are database changes. Warning Changes and additions to DNS Providers require verification by at least 2 members of the community! |
|
If you're interested, here's my rework patch: cf7306e |
|
I think this will error, you changed to snake_case only in the response. |
Shows a notification on the footer when a new version is available!
Demo:

PS: I don't believe it should be shown in the login screen, as it would only serve as an advertisement that it is an outdated version.