Mostly UI updates #495
Merged
Mostly UI updates #495
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Heatmap itself is still not responsive (a work in progress) - Heatmap no longer extends rightward beyond width of tabs - Updated some aesthetics of heatmap section (font sizes, labels' text) - Updated comments to be more instructive
- Shortened and centered title - Added expression value to y-axis - Updated font sizes
- Heatmap itself is still not responsive (a work in progress) - Heatmap no longer extends rightward beyond width of tabs - Updated some aesthetics of heatmap section (font sizes, labels' text) - Updated comments to be more instructive
- Shortened and centered title - Added expression value to y-axis - Updated font sizes
- Because, previously, these were getting cut off in the visualization
…ta explore section - Previously, many of these clinical features were represented as pie charts, which made the values difficult to read
- It does not contain any patient barcodes/expression data
- Removed 'date' and 'tool' because they yield the same value across all tumor types - Removed 'tcga_participant_barcode' because it does not align with the purpose of the data explore section at the moment
…ck/partition lists for heatmap and violin plots - For same reason as why they were removed from clinical feature select dropdown
This was
linked to
issues
Oct 18, 2024
ZhenghaoZhu
reviewed
Oct 19, 2024
main/js/fillSelectBoxes.js
Outdated
Comment on lines
338
to
369
| let continuousFeaturesArr = [ | ||
| "age_began_smoking_in_years", | ||
| "age_at_diagnosis", | ||
| "cervix_suv_results", | ||
| "date_of_initial_pathologic_diagnosis", | ||
| "days_to_death", | ||
| "days_to_last_followup", | ||
| "days_to_last_known_alive", | ||
| "days_to_psa", | ||
| "days_to_submitted_specimen_dx", | ||
| "gleason_score", | ||
| "height_cm_at_diagnosis", | ||
| "initial_pathologic_dx_year", | ||
| "karnofsky_performance_score", | ||
| "lymph_nodes_examined", | ||
| "lymph_nodes_examined_he_count", | ||
| "number_of_lymph_nodes", | ||
| "number_pack_years_smoked", | ||
| "pregnancies_count_ectopic", | ||
| "pregnancies_count_live_birth", | ||
| "pregnancies_count_stillbirth", | ||
| "pregnancies_count_total", | ||
| "pregnancy_spontaneous_abortion_count", | ||
| "pregnancy_therapeutic_abortion_count", | ||
| "psa_value", | ||
| "tobacco_smoking_history", | ||
| "tobacco_smoking_pack_years_smoked", | ||
| "tobacco_smoking_year_stopped", | ||
| "weight_kg_at_diagnosis", | ||
| "years_to_birth", | ||
| "year_of_tobacco_smoking_onset" | ||
| ]; // Array of features that should be considered continuous |
There was a problem hiding this comment.
This seems to be a constant which are normally placed in a constants.js file. Furthermore, constant names are capitalized and use snake case. In this case, this would let CONTINUOUS_FEATURES = []. No need to add "arr" as the features key word implies multiple items.
Comment on lines
+36
to
+39
| para.setAttribute( | ||
| "style", | ||
| 'text-align: center; color: #4db6ac; font-family: Georgia, "Times New Roman", Times, serif' | ||
| ); |
There was a problem hiding this comment.
This is similar to the code above
// build label:
let para = document.createElement("p");
// Style the paragraph
para.style.textAlign = 'center';
para.style.color = '#4db6ac';
para.style.fontFamily = 'Georgia, "Times New Roman", Times, serif';
para.id = "numSamplesInCohortText2";
Instead of doing this, you could create a function to set all of these attributes. This would be useful to make sure all attributes we want to modify are accounted for each time.
| @@ -298,7 +299,11 @@ let fillClinicalSelectBox = async function () { | |||
| } | |||
| } | |||
| } | |||
|
|
|||
|
|
|||
| const unwantedKeys = new Set(['date', 'tcga_participant_barcode', 'tool']); | |||
There was a problem hiding this comment.
It seems this set variable is declared in multiple places. It would be good practice to have a shared file instead of re-declaring the variable in each file every time.
enemeth19
requested review from
adit-anand and
ZhenghaoZhu
and removed request for
adit-anand
ZhenghaoZhu
approved these changes
Oct 29, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See commit messages for notes on which features have UI updates