-
Notifications
You must be signed in to change notification settings - Fork 17
Adjust Report Template for Skin Samples #345
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
Conversation
| site_specific_elements[i].style.display = ""; | ||
| } | ||
| // Hide Microbiome Maps for skin |
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.
We don't have any pcoa plots for skin, so we need to hide the links to that tab.
AmandaBirmingham
left a comment
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.
A grammar fix, a couple of strong suggestions (although I will accept counterarguments :), and some questions
| state.dataset_type.value = '16S'; | ||
| state.dataset_site.value = 'gut'; | ||
| state.dataset_input.value = 'tmi-16S-gut'; | ||
| state.dataset_site.value = body_site; |
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 like that state.dataset_site.value is now being set with a variable instead of hard-coded! Since the value being stored in state.dataset_input.value can be expressed as 'tmi-' + state.dateset_type.value + '-' + body_site , it would be great to go a step farther and move the line to set state.dataset_site.value = body_site and the alternate line above to set state.dataset_input.value OUT of the if/else, because they will be the same for both branches.
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.
Good catch, adjusted.
| // Hide Microbiome Maps for skin | ||
| if (state.dataset_site.value === "skin") { | ||
| document.getElementById("microbial-map-tab").parentElement.style.display = "none"; |
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.
Why can't the elements being handled in this if be assigned the site_specific_content class and hidden as part of the code directly above?
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.
Good catch, adjusted.
| <strong>{{ _('How do we calculate your diversity value?') }}</strong> | ||
| </p> | ||
| <p> | ||
| <p class="site_specific_content site_specific_content-gut"> |
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.
why switch from underscores to hyphen at the end of site_specific_content-gut? This seems likely to cause confusion ...
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.
Adjusted.
| </div> | ||
| <div class="col-sm-8 diversity-text"> | ||
| {{ _('<span id="beta_diversity_age_value" class="info-loader font-weight-bold">...</span> of people with a microbiome like yours were the same age as you') }} | ||
| {{ _('<span id="beta_diversity_age_value" class="info-loader font-weight-bold">...</span> of people with a microbiome like yours were a similar age as you') }} |
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.
grammatically, this would need to be "a similar age to you"
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.
Adjusted.
| {{ _('This is a table of all of the different microbes we found in your sample along with their proportions.') }} | ||
| </p> | ||
| <p class="site_specific_content site_specific_content-skin"> | ||
| {{ _('This is a table of all of the different bacteria we found in your sample along with their proportions.') }} |
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.
so the skin info includes only bacteria while the gut info includes microbes from other domains of life?
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 brought this edit to Se Jin's attention and we're going to harmonize both gut and skin to say "bacteria and archaea" rather than "microbes" for gut and "bacteria" for skin.
|
@AmandaBirmingham This is ready for re-review at your convenience, I believe I've either made all suggested changes or responded accordingly. |
This PR moves skin samples from the old template to the new template.