Review changes - #35
Conversation
| multiple = T, | ||
| shinyBS::bsCollapsePanel('Sample metadata (All)', style = 'primary', | ||
| DT::dataTableOutput('tblSmpMetadat') %>% | ||
| #### Collapsed panel: Workaround (SHOULD WORK BUT NOT WORK) |
There was a problem hiding this comment.
Just to be sure, did you do anything for improving the rendering in collapsed panels, or just delete these comments?
| reactMetabObj$metabObj <- NULL | ||
| } | ||
| }) | ||
| #### Collapsed panel: Workaround |
There was a problem hiding this comment.
The reason that I close the panel for sample metadata is to make the interface look cleaner. It works as the panel was initially open and close intentionally here. In addition, the content of this panel will not change through the processing and analysis (it always show all sample metadata even after sample removal).
Is it like it does not work on your machine?
There was a problem hiding this comment.
For me when uploading a new dataset, it sometimes shows the metadata table for the old dataset. Which was fixed after removing:
outputOptions(output, 'tblSmpMetadat', suspendWhenHidden = F)
But you are right, this then makes the interface very cluttered
There was a problem hiding this comment.
I think I misunderstood the comments. I thought they were about the wrong metadata for new data, but they were for the rendering inside a collapsed panel, so I will add them again!
| fluidRow( | ||
| style = 'display:flex; align-items: center;', | ||
| column(width = 2, | ||
| numericInput('pcxPCA', 'PC (x-axis)', |
There was a problem hiding this comment.
Minor suggestion: I think it would be better to use drop-down lists for PCs. Users then do not have to click many times or input numbers themselves to reach lower-order PCs. In addition, it can prevent users from input invalid numbers, i.e., non-integer, zero, negative values. Even so, you have explicitly dealt with these issues in the server part.
| row_vars <- apply(conc_mat, 1, function(x) var(x, na.rm = TRUE)) | ||
| se_pca <- se_pca[!is.na(row_vars) & row_vars > 0, ] | ||
|
|
||
| result <- MetaProViz::viz_pca( |
There was a problem hiding this comment.
Minor suggestion: Now, PCA reruns every time when the input for PCs is changed, which may cause a computation burden when a dataset gets larger. The better way would be separating PCA implementation and visualization. However, biocrates datasets should not be extremely large as it is a targeted approach. Therefore, I think we can keep it like this for now.
|
Thank you, Luis, for your great work! Please check my reviews, especially the one with intentional panel closing for sample metadata table. The others are minor and we may just keep it like this for now. |
2 participants
Implemented Changes from Reviewers