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

Web page proposal for Version 4.0.0 ESP32 #404

Closed

Conversation

Clemens-Toegel
Copy link

  • Change API path from AsynWebServer
  • Include new web page

As promised here is my redesign of the web page on the microcontroller.

- Change API path from AsynWebServer
- Include new web page
@fiendie
Copy link
Member

fiendie commented Dec 4, 2023

Sorry, not going to review this in its current state. It looks like 90% unnecessary whitespace changes. Please don't just hit format document in your editor.

@kjyv
Copy link
Collaborator

kjyv commented Dec 4, 2023

This deletes all of the existing webinterface in favor of one minified js file - how is this supposed to be maintained? (or even reviewed)

@Clemens-Toegel Clemens-Toegel marked this pull request as draft December 4, 2023 10:29
@Clemens-Toegel
Copy link
Author

Clemens-Toegel commented Dec 4, 2023

Hi,

Please provide a suitable .editorconfig so that the formatting is how it should suppose to be. I just used the auto format function of VS Code and this is the result.

You are right, this PR can't be reviewed properly. I have marked the PR as draft. If its okay I would leave the PR open to be used as a showcase and maybe for you to give feedback on the design of the web page, so we can see if you are at all interested in something like this.
I will also publish the code of the website (hopefully) soon on my Github, then you can see how the code is structured. I would comment the link on this PR when I am ready. Then, we we could discuss the next steps if you are interested. In the community chat we already discussed a few things and open questions need to be clarified, I summarise them here:

The proposal for the web page requires a different approach than the current implementation. That is why we need to decide where the source code of the web page should be available, because in the end only minified code should be in the data folder.

  1. We need either a new folder in this repo or a new repo only for the web page
    • Then, a proper documentation is needed, so that users can build the web page and use it with their ESP
  2. Should a default build be included or not in this repo in the data folder?
    • Of course, this would mean a minified code is checked in in this repo
  3. The chart implementation needs to be refactored, I have tried to copy your uplot implementation, but it seems that it does not work properly.

@kjyv
Copy link
Collaborator

kjyv commented Dec 4, 2023

I think there was discussion about the .editorconfig and that this is a local file. In any case, we have no automatic rules set up yet for formatting, so please don't use that feature in VS Code on complete files. The current quasi style we have has { after the line, not on a new line btw.
I think it is no problem to have the minified file in the data folder already, but it should be built automatically anyway when building and uploading to the ESP, so this needs to be set up. So possibly the default is not required. And the sources for that should IMO be in this repository, we would otherwise have to use the other repo as a submodule which likely causes issues and more questions from people in the chat.
I can't say anything about why uplot is special here but it would be nice if you could get it to work again (as it's fast and small, compared to plotly that we used before). Everything should still work in offline mode, i.e. all libraries need to be downloadable from the ESP.

@Clemens-Toegel
Copy link
Author

Clemens-Toegel commented Dec 4, 2023

The .editorconfig is already in the repo, so it can be edited to your configuration, so others use the style you want, I guess...

Okay, nice. Thanks for the input. That would be nice if it can be build automatically. I'm not very familiar with platform io, so I would definitely need your help there.
In the build everything is included, so the ESP does not need the internet for the webpage.

@fiendie
Copy link
Member

fiendie commented Dec 13, 2023

We added a CONTRIBUTING file as a reference for the code style. I think this is more flexible and allows for exceptions when they make sense.

@fiendie
Copy link
Member

fiendie commented Jan 14, 2024

@Clemens-Toegel: Are you still planning to publish the code? Otherwise I would close this PR.

@fiendie
Copy link
Member

fiendie commented Jan 14, 2024

After discussing this further, this is currently not a priority for the project. Feel free to open a new PR once the code is published and you have a detailed workflow for how to build and maintain the code.

@fiendie fiendie closed this Jan 14, 2024
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

Successfully merging this pull request may close these issues.

3 participants