-
Notifications
You must be signed in to change notification settings - Fork 107
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
Add link to next Webui #11
base: master
Are you sure you want to change the base?
Conversation
wifimgr.py
Outdated
@@ -6,6 +6,7 @@ | |||
ap_ssid = "WifiManager" | |||
ap_password = "tayfunulu" | |||
ap_authmode = 3 # WPA2 | |||
linkToNextWebinterface = True |
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.
style, see my comment in other PR.
wifimgr.py
Outdated
if linkToNextWebinterface: | ||
response += """\ | ||
<p style="text-align: center;"> | ||
<a href="http://%(ip)/">To new Interface</a><br> |
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.
did you mean %(ip)s
here?
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.
i reviewed this again and noticed that i did not really understand what the "next/new webui" actually means.
maybe this should be rephrased so that it is cleared what it actually does. variable name as well as UI content.
If |
Ah, can you just copy and paste your last comment to the source code, above the setting? If we do not display that new IP yet as plain text, that might be also useful for the cases the user does not run a web interface on the new ip, but some other service (but still needs to know the new ip somehow). |
wifimgr.py
Outdated
if link_to_next_webui: | ||
response += """\ | ||
<p style="text-align: center;"> | ||
<a href="http://%(ip)s/">To new Interface [%(ip)s]</a><br> |
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.
Thanks for updating this. But there is a slight problem now: the new IP is only shown IF link_to_next_webui is True (which only makes sense if you have a http web interface running on the new IP).
If you don't have a http web interface running there, you want to set this to False, but then it also won't display the IP address in plain text - but you might need it, if you run something else on that IP.
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.
My idea: Add an extra variable that if True
shows the IP on the success page and leave link_to_next_webui
as is. Do you agree ?
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.
Or just always show it, because "why not"?
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.
That's good, too.
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.
lgtm
Adds link to success page to IP in new WiFi.