Add progress ring to plots ribbon while data is loading #2841
Conversation
mattseddon
force-pushed
the
add-ribbon-progress-ring
branch
from
383e0ab to
a45aec3
Compare
|
This looks great! Makes things a little more dynamic. |
mattseddon
force-pushed
the
add-ribbon-progress-ring
branch
from
a45aec3 to
f1e790b
Compare
|
|
||
| useEffect(() => { | ||
| setOrder(pastOrder => performOrderedUpdate(pastOrder, revisions, reorderId)) | ||
| }, [revisions]) |
There was a problem hiding this comment.
[F] There was a race condition between the data update and useEffect that would cause reorderObjectList(order, revisions, reorderId) to drop selected checkpoint experiments from the ribbon. Deleting this means that we need to update the message passed for comparison plots and split the order of the comparison table columns from selectedRevisions (to reinstate => 'should not reorder the ribbon when comparison plots are reordered').
There was a problem hiding this comment.
I will do this as an immediate follow up
| @@ -1818,37 +1818,6 @@ describe('App', () => { | |||
| type: MessageFromWebviewType.REFRESH_REVISIONS | |||
| }) | |||
| }) | |||
|
|
|||
| it('should not reorder the ribbon when comparison plots are reordered', () => { | |||
There was a problem hiding this comment.
mattseddon
force-pushed
the
add-ribbon-progress-ring
branch
from
a48afa1 to
b8e07e5
Compare
| @@ -27,6 +26,11 @@ | |||
| display: inline; | |||
| font-size: 0.8125rem; | |||
| } | |||
|
|
|||
| .fetching { | |||
There was a problem hiding this comment.
[F] Gives us the space to display an icon against the experiment (e.g <Error/> if we have an issue with it)
| @@ -0,0 +1,61 @@ | |||
| import React from 'react' | |||
There was a problem hiding this comment.
[F] Add a story to demonstrate the styling of the ribbon with and without progress ring(s)
There was a problem hiding this comment.
I'm sure we'll need to iterate on the style.
|
Code Climate has analyzed commit e21e29d and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 100.0% (85% is the threshold). This pull request will bring the total coverage in the repository to 96.7% (0.1% change). View more on Code Climate. |
1/2
main<- this <- #2859Closes #2835.
This PR adds a progress ring to revisions in the plots ribbon whilst data is being loaded. This would have been a simple change had it not been for a race condition in running checkpoint experiments where the revision was never actually loaded until the experiment finished.
This change comes with a caveat (see inline comments). I will follow up straight away.
Demo
Screen.Recording.2022-11-30.at.3.44.31.pm.mov