SCT-3097 - fix clustering viewer pop ups, timeouts, tab crash with large clusters #2385
Conversation
Debug issue with narrative_version
Stage the 4.4.0 release
… size and the butchering of bootstrap styling.
…le missing feature type; minor code improvements
… by the table; provide explanation for heatmap download when genes > 50; disable heatmap for pairwise when > 200 genes (not invoked now since the button won't be displayed in this case.)
…discovering this typo.
…etID to get precise object ref - previously version was not included.
…separate out helper widgets, each with tests.
|
I'll be iterating next on the code quality reporting, will ping when ready for review. |
Codecov Report
@@ Coverage Diff @@
## develop #2385 +/- ##
===========================================
+ Coverage 73.25% 73.26% +0.01%
===========================================
Files 36 36
Lines 3903 3901 -2
===========================================
- Hits 2859 2858 -1
+ Misses 1044 1043 -1
Continue to review full report at Codecov.
|
|
SonarCloud Quality Gate failed.
|
Merge to main branch for 5.6.0 release
|
SonarCloud Quality Gate failed.
|
|
SonarCloud Quality Gate failed.
|
…aused them to be fixed, calculated width [SCT-3097]
…blem, the problem was usage of double= rather than triple=; and the solution was a better type test) [SCT-3097]
|
@ialarmedalien @briehl I'm going to be iterating on this, want to put it to bed. The main issues above I think are style -> stylesheet + classes, and css -> scss. I have small set of additions I could add, but haven't yet. In order to evaluate changing the inline styles to a stylesheet, I created a demo capability (5 files in static/kbase/demo and one change to narrative_paths) in order to evaluate the JSONVIew component. It is problematic to evaluate components which are only shown in certain situations - errors, fleeting moments like loading indicators, etc. |
this is necessary in order to validate modification of styles for components which are only displayed under certain circumstances (e.g. error conditions, loading, etc.)
|
@ialarmedalien I've moved the stylesheets and direct styles into scss, added the demo stuff. |
|
SonarCloud Quality Gate failed.
|
| @@ -0,0 +1,3 @@ | |||
| { | |||
| "extends": "stylelint-config-standard" | |||
There was a problem hiding this comment.
we've already got .stylelintrc.yaml so this shouldn't be needed
| @@ -0,0 +1,54 @@ | |||
| define([], () => { | |||
There was a problem hiding this comment.
can you integrate these functions into test/unit/testUtil.js?
There was a problem hiding this comment.
It looks like they were already copied into mswUtils at some point, so I'll extract from here and mswUtils and place into testUtil.js.
There was a problem hiding this comment.
I tend to think that testUtil is a bit too big and sprawling, though.
There was a problem hiding this comment.
Actually, waitFor in testUtil.js is not the same thing at all, so best not to incorporate there, or rename it.
There was a problem hiding this comment.
There is a "wait" in testUtil, but it is based on Bluebird, "waitFor" is plain JS so preferable.
I think it is best to keep these async utils separate, and to remove the duplicates from mswUtils, as they are unrelated to msw.
Fix clustering App causing persistent pop-ups in Narratives, viewing output stalls Narrative
This fixes behavior in
kbaseExpressionFeatureClusters, the top level viewer, andkbaseExpressionPairwiseCorrelation, a viewer for a tab view, in which large clusters can stall and even crash the browser with long data fetches and processing and error pop-ups, as reported by a user and confirmed.These are two distinct issues.
The first, the stalling and crashing, was due to attempting to create heat maps for large clusters, greater than about 200 genes.
The second, browser pop-ups, was due to changes in the KBaseGenomes.Feature workspace type between version 2.X and 3.0, in
The changes consisted of those distinct changes, and other changes in those and related files to enable unit testing.
First, in
kbaseExpressionFeatureClusters, thekbaseExpressionPairwiseCorrelationwill not be accessible with > 200 genes. This was accomplished by disabling the button which displayed the pairwise correlation heatmap tab. Not much point allowing a user to add a new tab which just displays a warningSecond,
kbaseExpressionPairwiseCorrelationwill just show a warning with > 200 genes (even though the button to access it is disabled). With > 50, <= 200 genes the heatmap is generated but not placed in the browser (this is a modification of existing behavior, which did not have the <= 200 cutoff), but is available for download. Below 50 genes the heatmap is generated and displayed.Jira Ticket / Issue
DATAUP-69 Adds a PR template)Testing Instructions
There are new tests for
kbaseExpressionPairwiseCorrelationand related sub-widgets (error, loading). These test include test data, derived from CI, which invoke the general conditions. Test coverage is pretty high, 100% for the subwidgets, and about 85% for the viewer widget. The untested code consists primarily of that which enables downloading of a heatmap when it cannot be rendered in the browser.Dev Checklist:
Updating Version and Release Notes (if applicable)