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

Build static #335

Merged
merged 6 commits into from
Feb 2, 2023
Merged

Build static #335

merged 6 commits into from
Feb 2, 2023

Conversation

surchs
Copy link
Contributor

@surchs surchs commented Feb 1, 2023

This PR sets up the master branch for deployment on GH pages and to use a custom domain https://annotate.neurobagel.org

Most steps are just direct from the docs: https://nuxtjs.org/deployments/github-pages/

I did some extra things:

  • removed the client-only / no-ssr tag on the annotation.vue page (not compatible with static build)
  • removed the custom route prefix that is suggested for GH deployments in the docs because we are using a custom domain

Commented out client-only tag in the annotation.vue page.
This tag was added as a workaround for a previous bug.
It seems to not be needed anymore. It will be fully removed
once the current refactor is completed
Added static build target and routing for GH pages
according to these docs
https://nuxtjs.org/deployments/github-pages/
To push to GH pages
To enable pushing to GH pages
We now use a custom domain so we don't need it anymore
fingers crossed
@surchs surchs requested a review from jarmoza February 1, 2023 13:42
Copy link
Contributor

@jarmoza jarmoza left a comment

Choose a reason for hiding this comment

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

@surchs Looks good. I just tested out the app. Very cool that this can exist statically.

All of the functionality seems to work with the exception of diagnosis annotation - where there is some wonkiness in some columns not getting annotated or getting wiped after being re-annotated.

Might be worth a brief looking into as I don't recall this problem in the master build.

@surchs
Copy link
Contributor Author

surchs commented Feb 2, 2023

Do you mean like this: #128 ?

@jarmoza
Copy link
Contributor

jarmoza commented Feb 2, 2023

No. Apologies for being unclear where this is happening. I mean on the Download page.

I had tried annotating for all categories just now (Subject ID, Age, Sex, Diagnosis (Group and GroupDX), Assessment Tools (1 group with IQ). I was seeing one and then both of the Diagnosis columns being completely clear. Retrying just now I believe I see where the bug is, however. It's when the Diagnosis category has its annotations saved more than once.

This is the result on the download page.

Screen Shot 2023-02-02 at 10 20 53 AM

All that said, I think this is a new-ish bug (similar to #176). I also note that the behavior of the 'Save Annotation' button is inconsistent across tabs (Does it gray out when saved to indicate a save has occurred or does it stay green?). I have filed issues for these two things. (See #339 and #340.)

I'm going to go ahead and approve this PR for merge.

@surchs
Copy link
Contributor Author

surchs commented Feb 2, 2023

Ah yeah, I think #176 is the better fit, agreed. That said: I think we encountered this behaviour/bug before and discussed that it will be resolved together with the larger refactor. If you think this is a new bug, then we should take another look, but not very urgently as the underlying codebase will (mostly) be replaced once the refactor branch merges.

@surchs surchs merged commit 095261b into master Feb 2, 2023
@surchs surchs deleted the build_static branch February 2, 2023 15:37
@jarmoza
Copy link
Contributor

jarmoza commented Feb 2, 2023

@surchs Agreed. We can revisit this at a later time - if it still exists. I've filed issues #339 and #340 for them.

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.

2 participants