ToggleNode should use Node.swapVisibility

[jessegreenberg]

From phetsims/plinko-probability#104, ToggleNode should manage focus for a11y. Node.swapVisibility was added to replace visibility of two Nodes and manage focus, so ToggleNode can make use of this.

[jessegreenberg]

@samreid since you are listed as an author would you mind reviewing?

[samreid]

On hold pending discussion in phetsims/scenery#694 (comment)

[jessegreenberg]

Discussion is complete in phetsims/scenery#694, we changed swapVisibility to look like node.swapVisibility( otherNode ). Visibility of node and otherNode cannot be equal, but otherwise there are no restrictions to which node is visible before calling the function. @samreid removing the 'on-hold' label but feel free to wait until final review of phetsims/scenery#694 is complete if you prefer.

[samreid]

ToggleNode will need to override dispose to avoid a memory leak because the constructor introduced a new listener to a non-owned Property.

I tested plinko probability with ?accessibility and it appeared that focus transferred properly to the grayed out play button when I pressed all + play.

[jessegreenberg]

Thanks @samreid, I added dispose to ToggleNode, and also added ToggleNode to MemoryTestsScreenView to test for ToggleNode leaks. Before adding dispose you can see a climbing JS Heap in between major garbage collections:

And after dispose was added that goes away:

@samreid can this be closed?

[samreid]

Looks great, thanks, closing.