phetsims / sun

User-interface components for PhET simulations, built on top of Scenery.
MIT License
4 stars 12 forks source link

Use DelayedMutate for AccessibleSlider stack #801

Closed jessegreenberg closed 7 months ago

jessegreenberg commented 1 year ago

AccessibleSlider trait pattern requires you to specify the index of options so that it knows where to put them when forwarding to the super call. DelayedMutate makes this unnecessary and it would be great if we could use it so that we don't have to specify the options position in the args. In order to use DelayedMutate the options all have to have a get/set function (like mutator keys). So this work would involve turning the options of AccessibleSlider stack into getters/setters so that it can use DelayedMutate.

jessegreenberg commented 7 months ago

I think this is done. Ill make sure before closing.

jessegreenberg commented 7 months ago

The point of this issue was to remove the optionsPosition from the constructor args since that has been a difficult part of using AccessibleSlider.

After discusing with @jonathanolson, DelayedMutate is not really related to optionsArgPosition. optionsArgPosition is required for AccessibleValueHandler because we need to set up state from the options during construction. That is generally not true for other Traits as state is set up directly via mutate.

We discussed an alternative where there is a initialize function instead of optionsArgPosition. But the client would need to remember to call that function. And that pattern won't work as nicely with DelayedMutate.

At this time, we think optionsArgPosition is the lesser of evils. Closing again.