SliderUnit should use nested options

[zepumph]

This is also the case for ohms-law, and I think they could both be implemented in a very similar way. @twant said she would investigate. Thanks!

Perhaps something like:

      a11yMapValue: value => Util.toFixedNumber( value, 2 ),
      keyboardStep: 1,
      shiftKeyboardStep: 0.01,
      startDrag: _.noop,
      sliderOptions:{

        trackFillEnabled: 'black',
        trackSize: new Dimension2( ResistanceInAWireConstants.SLIDER_HEIGHT - 30, 4 ),
        thumbSize: new Dimension2( 22, 45 ),
        thumbFill: '#c3c4c5',
        thumbFillHighlighted: '#dedede',
        tandem: tandem.createTandem( 'slider' ),
        startDrag: function( event ) {
          if ( event.type === 'keydown' ) {
            self.keyboardDragging = true;
          }
          options.startDrag && options.startDrag( event );
        },
        endDrag: function( event ) {
          self.keyboardDragging = false;
          options.endDrag && options.endDrag( event );
        },

        // physical values in this sim can have up to 2 decimals
        constrainValue: function( value ) {
          return Util.toFixedNumber( value, 2 );
        },

        // a11y
        roundToStepSize: true, // default keyboard step rounds to pedegogically useful values

        // a11y
        containerTagName: 'li',
        labelContent: labelContent,
        labelTagName: 'label'
      },
      endDrag: _.noop
    }, options );

[twant]

@zepumph I pushed some changes to accomplish this. Would love your review. I noticed two different ways to handle the startDrag and stopDrag options between RIAW and Ohm's Law. I kept both for now, but would particularly love your advice about whether to condense down to a single way to handle these options.

[zepumph]

Review:

• [ ] In RIAW/ControlPanel, you moved startDrag/endDrag into the sliderOptions, but those were actually meant to be top level. If you see the default sliderOptions.startDrag in RIAW/SliderUnit, it is wrapping and then calling options.startDrag. Currently sound is broken for this. To fix this I would recommend doing exactly what we did with NumberControl's constrainValue yesterday. See how it goes. I think that it is sloppy to have the option passed to SliderUnit, and then wrapped into the sliderOptions by SliderUnit. Instead SliderUnit should wrap it gracefully, without needing to define a new option for SliderUnit.
• [x] (this wasn't because of your work, but would be good to change). Most shiftKeyboardStep and keyboardStep declared in ControlPanel are the same as the default in SliderUnit, so they aren't needed.
• [x] In RIAW/SliderUnit it looks like we added the tandem options in the SliderUnit options. It wasn't there before this commit. It is likely that this is my fault because of how I described tandem to you yesterday. A good way to check your work here is to launch RIAW in phet-io "Standalone" mode through phetmarks. There, on launch we will get an error because the list of phet-io elements changed without our knowing (the assertion will be "api mismatches present"). This bullet is causing https://github.com/phetsims/resistance-in-a-wire/issues/211, and is causing failures on CT. I'll assign that issue over to you to make sure that fixes here fix that bug over there.
• [x] I like that a11yCreateAriaValueText has no usages in SliderUnit.js
• [x] Somehow the default a11yMapValue in OL/SliderUnit lost its dynamic decimalPlaces. We can just add that back in, since it is in a closure: a11yMapValue: value => Util.toFixedNumber( value, options.decimalPlaces ),

Overall things are looking really good. Thanks for taking the lead on this. Let me know if you have any questions.

[twant]

Just pushed a change that should hopefully resolve the CT error documented in #211. Apologies again for not fixing this earlier @zepumph @pixelzoom! I'll keep an eye on CT to make sure the error is resolved with this change.

[twant]

@zepumph regarding the startDrag and endDrag options, I've moved them back to the top level options. I think I understand the piece you're referring to from NumberControl (where we used options.delta in the sliderOptions), but would appreciate you checking over this change to confirm that it's consistent with the structure you were imagining.

In terms of gracefully wrapping the options, I'm not sure whether that favors the Ohm's Law or the RIAW handling of startDrag and stopDrag -- can we discuss Tuesday?

[zepumph]

n terms of gracefully wrapping the options, I'm not sure whether that favors the Ohm's Law or the RIAW handling of startDrag and stopDrag -- can we discuss Tuesday?

Yes indeed. I think that for the most part I was looking at RIAW and trying to make sure we didn't lose functionality. Now looking at OL, it is more idiomatic to what was done in NumberControl, and makes more sense because it only uses a single option. I will get back to you about the best pattern.

[zepumph]

Other changes look good.

[zepumph]

I think wrapping the options is the best holdover pattern until https://github.com/phetsims/scenery/issues/1016 or https://github.com/phetsims/sun/issues/536 are figured out.