Hotkeys and their help content are duplicated and spread out.

[pixelzoom]

I discovered this while working on adding a hotkey to keyboard help for Fourier in https://github.com/phetsims/fourier-making-waves/issues/93. MAL was used as the model for doing this. What I see is that the help content for a hotkey is not at all related to the actual implementation of a hotkey, so the info for a hotkey is spread out and duplicated.

An example from MAL.

MAL has keyboard help content for TimeControlNode, which uses PlayControlButton.

In PlayControlButton.js, the hotkey is implemented as Alt+K:

          globalKeyStateTracker.altKeyDown &&
          KeyboardUtils.isKeyEvent( event, KeyboardUtils.KEY_K )

In MoleculesAndLightKeyboardHelpContent.js, utility method KeyboardHelpSection.createPlayPauseKeyRow is used to create the keyboard help content:

const playPauseRow = KeyboardHelpSection.createPlayPauseKeyRow( pauseOrPlayShortcutString, pauseOrPlayShortcutDescriptionString, rowOptions );

It looks like this:

Looking at KeyboardHelpSection.createPlayPauseKeyRow, it hardcodes 'K':

  static createPlayPauseKeyRow( labelString, labelInnerContent, options ) {
    return KeyboardHelpSection.createGlobalHotkeyRow( labelString, labelInnerContent, new LetterKeyNode( 'K' ), options );
  }

Looking at pauseOrPlayShortcutDescriptionString, it hardcodes both 'Alt' and 'K':

      "pauseOrPlayShortcutDescription": {
        "value": "Pause or play action in Observation Window with Alt key plus K."
      },

So I've identified 3 places where the 'K' key is specified.

Now looking into KeyboardHelpSection.createGlobalHotkeyRow, it also hardcodes 'Alt' via TextKeyNode.alt():

  static createGlobalHotkeyRow( labelString, labelInnerContent, keyIcon, options ) {
    return KeyboardHelpSection.labelWithIcon(
      labelString,
      KeyboardHelpIconFactory.iconPlusIcon( TextKeyNode.alt(), keyIcon ),
      labelInnerContent,
      options
    );
  }

Now I've identified 3 places where the Alt key is specified.

This is not a scalable/sustainable way to specify hot keys and their help content. If I wanted to change this hotkey from Alt+K to Option-M, I'd have to find and change 6 places in the code.

[jessegreenberg]

This issue was slated for this iteration. To summarize the changes we have made:

• We created HotkeyData, a class where you can declare keys and other content for listeners, keyboard help, and documentation.
• All global KeyboardListeners and many other KeyboardListeners are now pulling keys from HotkeyData.
• Keyboard help dialog contents are pulling from Hotkey Data where possible.
• Functions in KeyboardHelpSectionRow have been replaced with fromHotkeyData, so you can easily create a row of content from HotkeyData.
• Documentation and examples in the alt input quickstart guide have been updated.
• The code review checklist has been updated to require using HotkeyData.
• As a bonus, HotkeyData is being used to generate documentation for used Hotkeys (https://github.com/phetsims/phet-info/issues/211)
• As a bonus, Hotkey and KeyboardListener now use Properties for key strings to be i18n friendly in the future.

[jessegreenberg]

The notes above complete this issue, closing.