Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add keyboard navigation #150

Open
jessegreenberg opened this issue Mar 17, 2017 · 54 comments
Open

add keyboard navigation #150

jessegreenberg opened this issue Mar 17, 2017 · 54 comments
Assignees

Comments

@jessegreenberg
Copy link
Contributor

@zepumph and @jessegreenberg are going to instrument keyboard navigation in build-an-atom, this issue is to track progress and document any issues or questions.

@jessegreenberg
Copy link
Contributor Author

We started with the ExpandCollapseButtons in the accordion boxes. But I think it would be more conventional if the accordion boxes behaved more like this:
https://www.w3.org/TR/wai-aria-practices-1.1/examples/accordion/accordion1.html

Focus should surround the accordion box title and button instead of just the button.

@zepumph
Copy link
Member

zepumph commented Mar 29, 2017

@jessegreenberg if you could look at the commits above briefly, they seem pretty safe to me, I just want to make sure since they are common code. Currently we have tab navigation for pretty much all objects but the particles. @jessegreenberg and I are going to work on that further tomorrow.

zepumph added a commit that referenced this issue Mar 31, 2017
zepumph added a commit to phetsims/shred that referenced this issue Mar 31, 2017
zepumph added a commit to phetsims/shred that referenced this issue Mar 31, 2017
zepumph added a commit to phetsims/shred that referenced this issue Mar 31, 2017
jessegreenberg added a commit that referenced this issue Mar 31, 2017
@zepumph
Copy link
Member

zepumph commented Apr 25, 2017

I tried to see about the escape bug. Not sure still why you can't select the particle again after it has gone back to the bucket.Here is a work in progress to refactor some logic out of the keydown event. I think I should try to search in the sim code for what happens on end drag with the mouse. There is logic that decides where to put the particle, but I can't find it.

    this.addAccessibleInputListener( {
      keydown: function( event ) {


        if ( self.selectingShellNucleusOptions ) {
          var isDownRight = event.keyCode === Input.KEY_DOWN_ARROW || event.keyCode === Input.KEY_RIGHT_ARROW;
          var isUpLeft = event.keyCode === Input.KEY_UP_ARROW || event.keyCode === Input.KEY_LEFT_ARROW;

          // if event was an arrow key
          if ( isDownRight || isUpLeft ) {
            if ( isDownRight ) {
              self.currentOptionIndex = ( self.currentOptionIndex + 1 ) % self.shellNucleusOptions.length;
            }
            else if ( isUpLeft ) {
              self.currentOptionIndex = self.currentOptionIndex - 1;
              if ( self.currentOptionIndex < 0 ) { self.currentOptionIndex = self.shellNucleusOptions.length - 1; }
            }

            var currentNode = self.shellNucleusOptions[ self.currentOptionIndex ];

            currentNode.focus();

            // Moving the particle to the current option
            self.currentlyDraggedParticle.destinationProperty.set( currentNode.shellNucleusHoverLocations );

            // Update the last focused node
            self.previouslyFocusedNode = currentNode;
          }

          // If key represents 'place' or 'end' condition
          else if ( event.keyCode === Input.KEY_ENTER || event.keyCode === Input.KEY_SPACE ||
                    event.keyCode === Input.KEY_TAB || event.keyCode === Input.KEY_ESCAPE ) {

            self.handleEndPlacement( event.keyCode );
          }
        }
      }
      // },
      // blur: function( event ){
      //   self.handleEndPlacement( event.keyCode );
      // }
    } );


    this.handleEndPlacement = function( keyCode ) {
      // If there is currently a particle that is being dragged
      if ( self.currentlyDraggedParticle && self.currentlySelectedBucketFront ) {


        // TODO: Make conditionals more efficient and easier to read.
        // if the nucleus was selected
        if ( self.currentOptionIndex === NUCLEUS_OPTION_INDEX || keyCode === Input.KEY_TAB ||
             keyCode === Input.KEY_ESCAPE ) {

          if ( keyCode === Input.KEY_TAB || keyCode === Input.KEY_ESCAPE ) {
            debugger;
            self.currentlySelectedBucketFront.bucket.addParticleFirstOpen( self.currentlyDraggedParticle, true );
          }
          else{

            var nucleusNode = self.shellNucleusOptions[ self.currentOptionIndex ];
            self.currentlyDraggedParticle.positionProperty.set( nucleusNode.shellNucleusHoverLocations );

          }

          self.currentlyDraggedParticle.userControlledProperty.set( false );

          // This is to help animate accessible drag
          self.currentlyDraggedParticle.isAccessibleControlled = false;

          // Remove focusability if there are no particles
          if ( self.atom.particleCountProperty.get() === 0 ) {
            self.focusable = false;
          }

          self.currentlyDraggedParticle = null;

          // // If tab was pressed then don't focus on the bucketFront again. Instead go to the next tab navigable element
          if ( keyCode !== Input.KEY_TAB ) {

            // TODO: Ensure that this is called after all key events meant for the particleAtom are finished. See https://github.com/phetsims/a11y-research/26
            // put focus back onto the bucketFront
            setTimeout( function() {
              self.currentlySelectedBucketFront.focus();
              self.currentlySelectedBucketFront = null;
            }, 100 );
          }
          else {
            self.currentlySelectedBucketFront = null;
          }
        }
        else {

          // No longer selecting the shell or nucleus to place
          self.selectingShellNucleusOptions = false;

          // Not the node for the nucleus, let the out shells handle their own electron selection
          var outerNode = self.shellNucleusOptions[ self.currentOptionIndex ];

          outerNode.chooseElectron( self.currentlyDraggedParticle, self.currentlySelectedBucketFront );
        }
      }
    };

zepumph added a commit that referenced this issue May 2, 2017
@zepumph
Copy link
Member

zepumph commented May 2, 2017

5/2 keyboard nav meeting:

  • (from last weeks) escape tab logic.
  • Reset all button isn't reseting the size of the inner focus highlight (it stays big)
  • Particles that are being placed should be above (z layer) the Focus highlight (especially for the nucleus.

@terracoda
Copy link

@jessegreenberg and @zepumph, is it the intention that once a grabbed particle is dropped that another one automatically animates out of the bucket, without the user actively hitting spacebar on the particle bucket?

@jessegreenberg
Copy link
Contributor Author

@terracoda I don't think so, is that a bug that is happening sometimes? I can't get that to happen on Chrome.

@zepumph
Copy link
Member

zepumph commented May 2, 2017

I am getting some of that weird selecting logic when I press the spacebar for a long time, or just hold it down in certain cases. @jessegreenberg, we should look into some of these more complicated bugs together soon. Thanks @terracoda.

@terracoda
Copy link

@jessegreenberg and @zepumph sorry I was using either Firefox or Safari, likely Firefox. I will re-test and explain better post more details. It's a bug :-)

@terracoda
Copy link

@jessegreenberg and @zepumph having the rings of the atom in the focus order currently will help me work out the interaction visually. The rings won't be in the tab order once we get this sorted out, but it's very useful right now.

@terracoda
Copy link

@jessegreenberg and @zepumph, the behaviour where a new particle is pulled from the bucket upon dropping a grabbed particle in a region (#150 (comment)) only happens in Firefox. I'm using Firefox 53, MacOS Sierra 10.12.3.

The interaction seems fine in Safari.

@zepumph
Copy link
Member

zepumph commented May 5, 2017

I worked on the tab/ escape problem a bit more today:

    this.addAccessibleInputListener( {
      keydown: function( event ) {
        console.log( 'Current Option index keydown: ' + self.currentOptionIndex);


        if ( self.selectingShellsAndNucleusOptions ) {
          var isDownRight = event.keyCode === Input.KEY_DOWN_ARROW || event.keyCode === Input.KEY_RIGHT_ARROW;
          var isUpLeft = event.keyCode === Input.KEY_UP_ARROW || event.keyCode === Input.KEY_LEFT_ARROW;

          // if event was an arrow key
          if ( isDownRight || isUpLeft ) {
            if ( isDownRight ) {
              self.currentOptionIndex = ( self.currentOptionIndex + 1 ) % self.shellNucleusOptions.length;
            }
            else if ( isUpLeft ) {
              self.currentOptionIndex = self.currentOptionIndex - 1;
              if ( self.currentOptionIndex < 0 ) { self.currentOptionIndex = self.shellNucleusOptions.length - 1; }
            }

            var currentNode = self.shellNucleusOptions[ self.currentOptionIndex ];

            currentNode.focus();

            // Moving the particle to the current option
            self.currentlyDraggedParticle.destinationProperty.set( currentNode.shellNucleusHoverLocations );

            // Update the last focused node
            self.previouslyFocusedNode = currentNode;
          }

          // If key represents 'place' or 'end' condition
          else if ( event.keyCode === Input.KEY_ENTER || event.keyCode === Input.KEY_SPACE ||
                    event.keyCode === Input.KEY_TAB || event.keyCode === Input.KEY_ESCAPE ) {

            self.handleEndPlacement( event.keyCode );
          }
          console.log( 'Current Option index: bottom of keydown' + self.currentOptionIndex);

        }
      }
      // },
      // blur: function( event ){
      //   self.handleEndPlacement( event.keyCode );
      // }
    } );


    this.handleEndPlacement = function( keyCode ) {
      // If there is currently a particle that is being dragged
      if ( self.currentlyDraggedParticle && self.currentlySelectedBucketFront ) {


        // TODO: Make conditionals more efficient and easier to read.
        // if the nucleus was selected
        if ( self.currentOptionIndex === NUCLEUS_OPTION_INDEX || keyCode === Input.KEY_TAB ||
             keyCode === Input.KEY_ESCAPE ) {

          //

          if ( keyCode === Input.KEY_TAB || keyCode === Input.KEY_ESCAPE ) {
            self.currentlySelectedBucketFront.bucket.addParticleFirstOpen( self.currentlyDraggedParticle, true );
          }
          else{

            var nucleusNode = self.shellNucleusOptions[ self.currentOptionIndex ];
            self.currentlyDraggedParticle.positionProperty.set( nucleusNode.shellNucleusHoverLocations );

          }
          self.currentlyDraggedParticle.userControlledProperty.set( false );


          // This is to help animate accessible drag
          self.currentlyDraggedParticle.isAccessibleControlled = false;

          // Remove focusability if there are no particles
          if ( self.atom.particleCountProperty.get() === 0 ) {
            self.focusable = false;
          }

          self.currentlyDraggedParticle = null;

          // // If tab was pressed then don't focus on the bucketFront again. Instead go to the next tab navigable element
          if ( keyCode !== Input.KEY_TAB ) {

            // TODO: Ensure that this is called after all key events meant for the particleAtom are finished. See https://github.com/phetsims/a11y-research/26
            // put focus back onto the bucketFront
            setTimeout( function() {
              self.currentlySelectedBucketFront.focus();
              self.currentlySelectedBucketFront = null;
            }, 100 );
          }
          else {
            self.currentlySelectedBucketFront = null;
          }
        }
        else {

          // No longer selecting the shell or nucleus to place
          self.selectingShellsAndNucleusOptions = false;

          // Not the node for the nucleus, let the out shells handle their own electron selection
          var outerNode = self.shellNucleusOptions[ self.currentOptionIndex ];

          outerNode.chooseElectron( self.currentlyDraggedParticle, self.currentlySelectedBucketFront );
        }
      }
      else{
        console.log( 'we arenot choosing a shell/nucleus placement option right now.');
      }
    };

zepumph added a commit to phetsims/shred that referenced this issue May 5, 2017
zepumph added a commit to phetsims/shred that referenced this issue May 5, 2017
zepumph added a commit to phetsims/scenery that referenced this issue May 5, 2017
@zepumph
Copy link
Member

zepumph commented May 5, 2017

I got a lot done with @jessegreenberg this afternoon. We got stumped on the above escape/tab logic. Not really sure how to proceed, but otherwise there are just a few more things on our list right now (except for removing particles)

  • The X disappears for weird reasons, not sure what the logic is
  • Tab/escape logic when repopulating the sphere buckets again.

@zepumph
Copy link
Member

zepumph commented May 8, 2017

@jessegreenberg I just found a large problem with the most recent implementation of layering. Since we are adding focusHighlights as children, they are showing up in the sim (visible) when accessibility is not actively accessible enabled (&accessibility).

@emily-phet
Copy link

@zepumph @jessegreenberg We're not having the keyboard navigation meeting this week, but if it would be helpful to get some interim feedback, or help with some of the issues you're running in to, let me know. I can take a look.

@zepumph
Copy link
Member

zepumph commented May 8, 2017

For me, I think most of the difficulties we are running into are implementational rather than design focused, but thank you! We will keep you posted if feedback would be helpful.

@terracoda
Copy link

@zepumph, I worked on the PDOM document for Build an Atom today. Do you think it would be useful to start separate issue that focus on general structure? I've made some initial choices, but would like to see what people think.

@terracoda
Copy link

@zepumph, actually I am just going to start a new issue, even though structure is related keyboard navigation. I think it warrants a new issue.

@zepumph
Copy link
Member

zepumph commented Aug 14, 2017

Unassigning until we work on BAA again.

@zepumph zepumph removed their assignment Aug 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants