Decouple ribbon from comparison table revisions #1844
Conversation
mattseddon
force-pushed
the
update-plot-message
branch
from
48a32ea to
80c64c4
Compare
|
|
||
| useEffect(() => { | ||
| setOrder(pastOrder => performOrderedUpdate(pastOrder, revisions, reorderId)) | ||
| }, [revisions]) |
There was a problem hiding this comment.
@sroy3 I copied the pattern that we had elsewhere. Does this work for you?
There was a problem hiding this comment.
If we don't do that, it'll keep re-ordering with the comparison table? If so, that's great. If not, then I don't see why we need it.
|
|
||
| useEffect(() => { | ||
| setOrder(pastOrder => performOrderedUpdate(pastOrder, revisions, reorderId)) | ||
| }, [revisions]) |
There was a problem hiding this comment.
If we don't do that, it'll keep re-ordering with the comparison table? If so, that's great. If not, then I don't see why we need it.
| @@ -50,7 +59,7 @@ export const Ribbon: React.FC<RibbonProps> = ({ revisions }) => { | |||
| appearance="secondary" | |||
| /> | |||
| </li> | |||
| {revisions.map(revision => ( | |||
| {reorderObjectList(order, revisions, reorderId).map(revision => ( | |||
There was a problem hiding this comment.
This should have already been done by the useEffect above.
There was a problem hiding this comment.
Do I need to remove this?
There was a problem hiding this comment.
I didn't think that performOrderedUpdate mutated the original array?
There was a problem hiding this comment.
Sorry, I thought it was the same function twice. It's fine.
|
Code Climate has analyzed commit 7a33da1 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.9% (0.0% change). View more on Code Climate. |
This PR fixes two bugs:
Demo
Screen.Recording.2022-06-07.at.10.29.03.am.mov