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

Overlay plot for comparing cyclic voltammetry #178

Draft
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

linuxrider
Copy link
Contributor

@linuxrider linuxrider commented Jun 25, 2022

The idea is to have a plotly plot and the data is pulled from csv on the server on demand on selection.

On the bottom of the CV page there is now a compare link, which leads to a overview page.
On this the desired CVs can be selected for overlaying them.
In the current state there is a mysterious delay with the selection. The previous selection (i.e. selection on click in the past) is displayed. Probably related to promises.

Addresses #177.

@linuxrider
Copy link
Contributor Author

When using promises in a different way it works, i.e. the delay is gone.

@linuxrider linuxrider marked this pull request as ready for review June 26, 2022 06:21
@DunklesArchipel
Copy link
Member

When I test this locally, I see the graph but upon selecting data it is not updated. :(

@linuxrider
Copy link
Contributor Author

linuxrider commented Jun 27, 2022

@DunklesArchipel I forgot to mention that one needs to set an environment variable.
I will change the behavior so that this is not necessary anymore.

Ok, mkdocs serve should be enough with the change.
I really like how fast this is locally. Curious how it performs on echemdb.org.

@DunklesArchipel
Copy link
Member

Works now. I just wonder if the table in its current form is so useful. It would probably be good to have a list with identifiers to select from.

And lets see how this works when the number of entries grows even more.

@linuxrider
Copy link
Contributor Author

Of course, the selection mechanism/table can be improved. It was not the focus for now since it should just somehow work at first.
We can discuss this further.

Comment on lines +45 to +46
async function updatePlot(names) {
Promise.all(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
async function updatePlot(names) {
Promise.all(
function updatePlot(names) {
return Promise.all(

@saraedum saraedum marked this pull request as draft February 20, 2023 22:56
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