phetsims / build-an-atom

"Build an Atom" is an educational simulation in HTML5, by PhET Interactive Simulations.
http://phet.colorado.edu/en/simulation/build-an-atom
GNU General Public License v3.0
11 stars 10 forks source link

add keyboard navigation #150

Open jessegreenberg opened 7 years ago

jessegreenberg commented 7 years ago

@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 commented 7 years ago

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 commented 7 years ago

@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.

jessegreenberg commented 7 years ago

@zepumph and I took an initial pass at drag and drop for the nucleons in this sim, here is a version where it can be tried out!

http://www.colorado.edu/physics/phet/dev/html/build-an-atom/1.3.0-keyboard-nav.1/build-an-atom_en.html?ea&accessibility

@emily-phet @terracoda what do you think?

terracoda commented 7 years ago

@jessegreenberg and @zepumph, what browsers are you testing in. I seem to be getting different behaviour in Firefox and Safari. I just want to make sure I am seeing something similar to you :-)

terracoda commented 7 years ago

@jessegreenberg and @zepumph, and I should say it is looking interesting!!

I have a side question about the accordions... Can we nest the buttons in a headings? That will provide some nice structure for non-visual users?

I am working on a first pass for the heading outline.

jessegreenberg commented 7 years ago

Thanks @terracoda! Good point, we have only tested in Chrome so far, behavior might be different in other browsers. I will check it out.

Also, the above link doesn't support grabbing particles in the atom and placing them elsewhere or returning them to the bucket. We wanted to make sure we were headed down the right path before spending more time implementing.

Can we nest the buttons in a headings?

Hmm, I am not sure @terracoda, could you provide an HTML example so I can see what you mean?

jessegreenberg commented 7 years ago

https://github.com/phetsims/build-an-atom/issues/150#issuecomment-290249840

Commits look great @zepumph, thanks!

terracoda commented 7 years ago

@jessegreenberg, yes working on structure wireframe in a google doc BAA PDOM. I am not quite done yet. I can provide proper code after.

terracoda commented 7 years ago

@jessegreenberg and @zepumph, I completed a structural wireframe for Build an Atom which includes all Static Descriptions (headings, labels, and help text). I've asked @emily-phet for feedback.

The doc has two outline examples. If folks prefer example 2 (I think it is better), I'll have to update (re-arrange) the wireframe for the HTML.

terracoda commented 7 years ago

@jessegreenberg and @zepumph, I am curious to know how closely you have followed the design pattern in comment https://github.com/phetsims/build-an-atom/issues/150#issuecomment-287410837. The W3C example uses a definition list (DL, DT, DD) to house the accordion headings (and buttons) and accordion content. This is nice because the list is grouped and the DT's and DD's have an explicit relationship.

Headings and a container can also work, but do not have a natural top-level grouping. See example from HeydonWorks Practical Examples.

It would be nice to compare how these two patterns sound in an interactive sim.

terracoda commented 7 years ago

Regarding comment https://github.com/phetsims/build-an-atom/issues/150#issuecomment-290943362 and nesting buttons inside headings. Here's what I am thinking, but stripped down:

<h3>Atom Details</h3>   
<h4 aria-expanded=”true”>
    <button>Element in Periodic Table</button>
</h4>
<div aria-hidden="false">
     collapsible semantically structured content.
</div>

<h4 aria-expanded=”false”>
    <button>Net Charge</button>
</h4>
<div aria-hidden="true">
     collapsible semantically structured content.
</div>

<h4 aria-expanded=”false”>
    <button>Mass Number</button>
</h4>
<div  aria-hidden="false">
     collapsible semantically structured content.
</div>
jessegreenberg commented 7 years ago

I am curious to know how closely you have followed the design pattern in comment #150 (comment).

Not too closely @terracoda, and we can certainly use a definition list! We haven't yet because our focus for now was just keyboard navigation.

Regarding https://github.com/phetsims/build-an-atom/issues/150#issuecomment-291101570, that looks good @terracoda, we can do that.

terracoda commented 7 years ago

@jessegreenberg, a definition list would be similar, but has a natural built-in grouping. The dl's terms (Element, Net Charge and Mass Number) are somewhat defined by their associated values, and also define the Atom. It may be a useful design pattern both structurally and pedagogically.


<dl>
<dt aria-expanded=”true”>
    <button>Element in Periodic Table</button>
</dt>
<dd aria-hidden="false">
     collapsible semantically structured content.
</dd>
<!-- more collapsed items -->
</dl>
terracoda commented 7 years ago

@jessegreenberg, in case I was not clear, I think it is worth trying a definition list. I'll add a note in the design doc.

Would it be useful for you and @zepumph for me to create a stripped down example PDOM with real html (but no javascript)?

terracoda commented 7 years ago

@jessegreenberg, @zepumph the Grab particle buttons and Drop location Listbox work nicely from my perspective . I only did informal testing in Chrome with ChromeVox. There were differences in Firefox and Safari, but I have not documented that.

My notes are in the same google doc that has @emily-phet 's notes. See heading Testing Notes/Chrom (TS).

I found only one serious keyboard issue relating to focus on the listbox items. Focus is currently starting on last drop location rather than on when the particle is put upon the Grab interaction (i.e., particle is in Nucleus, but focus could be on Outer Electron ring).

Other issues:

Great work, so far!

jessegreenberg commented 7 years ago

Thanks @terracoda! We will take a look at the bugs mentioned in todays meeting, and work on getting it platform consistent.

jessegreenberg commented 7 years ago

@emily-phet @zepumph @terracoda @oliver-phet @ariel-phet @arouinfar @amyh-phet @amanda-phet here are some notes from the meeting today. Please feel free to modify or add if anything is missing or incorrect:

terracoda commented 7 years ago

@jessegreenberg, these are great notes. I also took notes in the google doc that @emily-phet shared.

zepumph commented 7 years ago

Escape key and tabbing away from a dragging interaction should move particles back into the bucket?

Note: There wasn't a clear consensus on this one

I think that this is easy enough to do, so we should try it out and see what people think for next week's meeting.

jessegreenberg commented 7 years ago

@emily-phet was curious if the user needs the ability to move an electron to a specific spot in an electron orbital. Content wise, it is important to be able to place a particle in a particular spot in the orbit.

Perhaps dropping the particle the first time should work more like moving the particle from one position in the atom to another?

terracoda commented 7 years ago

@emily-phet @zepumph @terracoda @oliver-phet @ariel-phet @arouinfar @amyh-phet @amanda-phet Regarding grouping things in the PDOM. I think we need to have a good reason to group things - reasons that align with the visual design and the learning goals.

Grouping things in the PDOM can have impacts on how keyboard navigation works, how outlines are created by AT. It also impacts how objects are labeled, and what kind of help text is provided.

Keep in mind that when using a screen reader, a user can already use the arrow keys to go to the next thing (e.g., bucket). While using the Arrow keys, however, the active keyboard focus highlight does not change. With screen reader use there are two kinds of focus, keyboard focus and virtual focus. If a user is not on a control (e.g., a grid, or listbox) that requires the use of the arrow keys, the Arrow keys allow them to read things without moving their keyboard focus. On controls such as listboxes and grids, their keyboard focus does change. For these controls, the focus change is expected.

I hope I am being clear here :-) While I like the idea of being able to use the Arrow keys to go to the next bucket, a screen reader user can already do this, and it may not be intuitive to people who use keyboard navigation regularly.

I think the buckets are nice as Top-level interactive objects. Other visual groupings or more complex sims may offer more relevant grouping design patterns.

Is this a case, of "if it ain't broke, don't fix it"?

jessegreenberg commented 7 years ago

That is fine with me @terracoda, we will not group the buckets for now.

emily-phet commented 7 years ago

@terracoda I completely agree here. It might be helpful for us (going forward) to be more explicit when we're mapping to a specific interaction structure - e.g., mapping a sim scenario to a more common website interaction. We can talk more about this later.

zepumph commented 7 years ago

On escape and tab, the particle will go back to the bucket, but it is difficult to animate that. Or at least I was having trouble with it. Perhaps there is something I'm not thinking of, but the reason is that all animation is done by setting the userControlledProperty to false, but we don't want to do that because it will just go into the model.

zepumph commented 7 years ago

For example, changing ElectronShellView:222 ish to:

        particle.userControlledProperty.set( false );

        // Put the particle back in the bucket if it isn't purposefully dropped.
        if ( keyCode === Input.KEY_TAB || keyCode === Input.KEY_ESCAPE ){
          particle.destinationProperty.set( bucketFront.bucket.getFirstOpenLocation());
        }

Yields some pretty weird results. I haven't had time to investigate yet.

jessegreenberg commented 7 years ago

@zepumph, it looks like your commit in https://github.com/phetsims/shred/commit/f9e68ab2e32ec8a3906298e19e54bc8a575fac7a should handle animating particles back into the bucket nicely, SphereBucket.addParticleFirstOpen takes a second param that lets you animate.

jessegreenberg commented 7 years ago

Feedback from 4/11/17 meeting:

oliver-phet commented 7 years ago

Also - if you have a particle selected and hovering a drop area, if you click to a different screen the particle remains in an odd state. I think switching screens should trigger escape behavior.

terracoda commented 7 years ago

Also regarding groupings in https://github.com/phetsims/build-an-atom/issues/150#issuecomment-291797772, I added section 7 to the Keyboard Navigation google document.

terracoda commented 7 years ago

@emily-phet, could you summarize in a short paragraph what kind of design documentation you are after? My pencil scratches are a little vague.

terracoda commented 7 years ago

@emily-phet, sorry maybe we should start a new issue in the a11y-research repo for this topic?

zepumph commented 7 years ago

these are the last shas working on a single node changing focus highlight. shred: 1f75a91154e8bddfc743c6e9aa25ba11ed393716 build-an-atom: 8917ae0e0bbfac889a5626c408cc4d381c83206a

jessegreenberg commented 7 years ago

Feedback from the meeting:

terracoda commented 7 years ago

@jessegreenberg, I suggest much smaller squares, almost like notches. With the addition of the pink focus highlight outlining the notch, the notch becomes bigger, making it standout visually from a non-focused spot.

jessegreenberg commented 7 years ago

Thanks @terracoda sounds good, I updated the list.

zepumph commented 7 years ago

One thought. Is it confusing that outer shells have another selection step, but the nucleus doesn't?

terracoda commented 7 years ago

@zepumph, there is no sub-navigation steps on the nucleus, so I would hope not.

zepumph commented 7 years ago

From 4/25 keyboard nav meeting:

zepumph commented 7 years ago

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 commented 7 years ago

5/2 keyboard nav meeting:

terracoda commented 7 years ago

@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 commented 7 years ago

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

zepumph commented 7 years ago

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 commented 7 years ago

@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 commented 7 years ago

@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 commented 7 years ago

@jessegreenberg and @zepumph, the behaviour where a new particle is pulled from the bucket upon dropping a grabbed particle in a region (https://github.com/phetsims/build-an-atom/issues/150#issuecomment-298746659) only happens in Firefox. I'm using Firefox 53, MacOS Sierra 10.12.3.

The interaction seems fine in Safari.

zepumph commented 7 years ago

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 commented 7 years ago

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)

zepumph commented 7 years ago

@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 commented 7 years ago

@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.