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

WebUI Enhancements and network setup page #42

Open
wants to merge 31 commits into
base: dev
Choose a base branch
from
Open

Conversation

Orbitix
Copy link
Member

@Orbitix Orbitix commented Jan 19, 2024

Adds a new page under network called 'setup', which contains a form that interacts with wlanpi-core api to set the wpa-supplicant. future UI changes will be made to make it look better and less crowded.

Below it there is an iframe that shows a table from the interface scan results. this is asynchronous so the page loads and doesn't wait for this first.

There is also a pill in the navbar that shows the current mode that the wlanpi is in, in the future this will be a selector to change the mode from the UI, which I have already designed but commented out.

@joshschmelzle joshschmelzle self-requested a review January 19, 2024 14:53
@joshschmelzle joshschmelzle self-assigned this Jan 19, 2024
@joshschmelzle joshschmelzle added the enhancement New feature or request label Jan 19, 2024
@joshschmelzle
Copy link
Member

@Orbitix network_setup.html template is missing. Can you add?

@Orbitix
Copy link
Member Author

Orbitix commented Jan 26, 2024

@joshschmelzle html file added. should work now!

@joshschmelzle
Copy link
Member

@Orbitix It looks like netscan_iframe.html is missing as well. Can you pull 280936f and then add netscan_iframe.html and push?

jinja2.exceptions.TemplateNotFound: netscan_iframe.html 

@Orbitix
Copy link
Member Author

Orbitix commented Jan 27, 2024

@joshschmelzle I am away this weekend so I can't push anything untill Sunday.

The git repo on my pi didn't pick up new files, only the modified ones so that's why so many are missing.

@Orbitix Orbitix changed the title New network stats page and set network form WebUI Enhancements and network setup page Sep 16, 2024
@bentumbler
Copy link
Contributor

Is there more to be done to this to prepare for merge

@joshschmelzle
Copy link
Member

@Orbitix is there anything on your side remaining to commit? I can look early next week and get this merged for Prague.

@Orbitix
Copy link
Member Author

Orbitix commented Oct 1, 2024

@joshschmelzle everything on my end is commited and working. The code probably just needs to be cleaned up a bit, but other than that it should be fine

@Orbitix
Copy link
Member Author

Orbitix commented Oct 4, 2024

@joshschmelzle I have pushed the change that fixes the webui crash when nginx times out, so it will just return "Internal Server Error" in the message box on the network setup page instead of just going to a 500 page.

Is there anything else that needs doing with webui or is it all good to hand over to you?

@Orbitix Orbitix self-assigned this Oct 4, 2024
@joshschmelzle
Copy link
Member

I can take a look either late tonight or tomorrow

@joshschmelzle
Copy link
Member

joshschmelzle commented Oct 8, 2024

Hey @Orbitix, thanks for taking the time on this PR. I started reviewing your PR last night and noticed a few issues that are causing errors and can easily break when used in production.

There are quite a few areas that need polishing, and because of the size of the changes, it's taking me longer than expected to go through everything in detail. I really appreciate the effort you're putting in, but as it stands, the code isn’t quite ready to be merged.

Before we move forward, I wanted to ask—how comfortable are you with receiving feedback and making revisions on this PR? I'd be happy to go through the changes with you and guide you on how to improve them. However, this will certainly delay these changes getting into the Prague release. Alternatively, if you'd prefer, I can take it over from here and incorporate the changes myself.

Let me know how you'd like to proceed, and thanks again for all your effort on this.

@Orbitix
Copy link
Member Author

Orbitix commented Oct 8, 2024

Thanks for the response @joshschmelzle, I would prefer if we could go through the issues together, as I feel it is valuable for me to learn how to resolve these kind of problems, and also prevent them in the first place as this is all still quite new to me, and I'm not very confident in my code as I don't completely know what I'm doing when it comes to robust and production-ready code.

I would also like the changes to be in the Prague release, as they link with Ben's core changes, so if needed I am fine with you taking over and finishing it if we run out of time.

I am also really grateful for your support and time with helping me learn and become a better coder, it is really valuable experience to learn how contributing to an active project works, and for me and any future jobs I may be developing in.

I may not be able to respond with changes very quickly because of school and studies, but I will whenever I have time.
I could give it a go at the weekend if I had a list by Friday, as that's when I have the most time to work on it.

@joshschmelzle
Copy link
Member

As discussed on the team meeting today, I plan on making some more changes to this PR and also in the process attempt to set some guidelines on how to handle async scenarios with htmx.

@Orbitix
Copy link
Member Author

Orbitix commented Oct 12, 2024

Alright thanks, that would help a lot as I don't really know how to use htmx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants