Going back to previously completed collections do not display said different collection

[loganbraywork]

Test device
Windows 10 Laptop
Operating System
Windows 10
Browser
Chrome
Problem description
From phetsims/qa#459

After a molecule collection is completed, it is possible to navigate back to said completed collection using the arrow button near the top of the collection box. Navigating back to previous collections does not alter the box in any way and the box remains on the current collection while the collection number changes.
Steps to reproduce

1. Navigate to Multiple screen
2. Complete the first collection
3. Attempt to Navigate back to the first collection from the second collection using the yellow arrow at the top of the collection box.
   Visuals
   

Troubleshooting information:
!!!!! DO NOT EDIT !!!!!
Name: ‪Build a Molecule‬
URL: https://phet-dev.colorado.edu/html/build-a-molecule/0.0.0-dev.23/phet/build-a-molecule_all_phet.html
Version: 0.0.0-dev.23 2019-11-20 23:29:39 UTC
Features missing: generatedcontent, touch
Flags: pixelRatioScaling
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/78.0.3904.97 Safari/537.36
Language: en-US
Window: 1536x754
Pixel Ratio: 1.25/1
WebGL: WebGL 1.0 (OpenGL ES 2.0 Chromium)
GLSL: WebGL GLSL ES 1.0 (OpenGL ES GLSL ES 1.0 Chromium)
Vendor: WebKit (WebKit WebGL)
Vertex: attribs: 16 varying: 30 uniform: 4095
Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 32)
Max viewport: 32767x32767
OES_texture_float: true
Dependencies JSON: {}

[loganbraywork]

If a molecule in the second collection is completed before moving back to the first then back to the second, the atoms used to complete said model will return and can be manipulated.

[KatieWoe]

Example of #121 (comment) as well as graphical issues that occur as a result.

[loganbraywork]

If an atom is released in the basket while on a previously completed collection screen the sim crashes. The console error is:

build-a-molecule_all_phet.html:1068 Uncaught TypeError: Cannot read property '_parents' of undefined
    at C.removeChildWithIndex (build-a-molecule_all_phet.html:1068)
    at C.removeChild (build-a-molecule_all_phet.html:1068)
    at t.value (build-a-molecule_all_phet.html:1068)
    at Array.<anonymous> (build-a-molecule_all_phet.html:1068)
    at C._fireItemRemoved (build-a-molecule_all_phet.html:1068)
    at C.remove (build-a-molecule_all_phet.html:1068)
    at C.recycleAtomIntoBuckets (build-a-molecule_all_phet.html:1068)
    at build-a-molecule_all_phet.html:1068
    at Array.forEach (<anonymous>)
    at C.recycleMoleculeIntoBuckets (build-a-molecule_all_phet.html:1068)

[Denz1994]

Commit dc895aa might have fixed this. Can you confirm this is fixed in this dev version @KatieWoe ?

Note: You can use the query parameter ?easyMode. It will allow you to go to the next collection after one completed molecule. You will need to refresh the sim if you don't go to next collection though.

[KatieWoe]

The issue and #121 (comment) both seem fixed. #121 (comment) still occurs and seems odd, but it does not cause issues anymore that I can see.

[Denz1994]

@arouinfar What do you think about this behavior in #121 (comment)? Do we want to keep the atoms in the play area as the user swaps between collections?

I personally vote to reset the play area an return the atoms to their buckets when the user swaps collections (the current behavior). I think of the collections as different scenes on a screen and refer back to how the masses are treated in MAS and MASB. Molecule and Shapes also reset molecules when swapping between molecules on the Real Molecules screen.

[arouinfar]

Do we want to keep the atoms in the play area as the user swaps between collections?

In Java the atoms remain in the play area when scrolling through the collections. It would be nice if we could keep that behavior, as it seems more natural.

That said, if it would take significant time investment or would make the code less robust/maintainable to do, I would be fine with resetting the play area when moving between collections.

[Denz1994]

The current approach was partly done because we don't have a limit on the number of possible collections for the collection screens.

This wouldn't be a straight-forward change because we are disposing of the listeners when the collections are being changed. We would need to track the listeners for each kit and reassign them, which is possible.

This will increase the memory profile as more collections are completed and then added, but I would need to do a headshot comparison to determine by how much.

[Denz1994]

During an impromptu meeting, @arouinfar and I thought that we should leave the behavior the way it is. Aside from the complexity described above (#121 (comment)), @arouinfar mentioned that limiting the number of collections would require a separate UI for completing the last collection.

[Denz1994]

From meeting on 02/07/20 @ariel-phet mentioned:

This seems fine to me.

We'll proceed with the current behavior, closing this issue.