phetsims / sun

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

generalize orientation in AquaRadioButtonGroup #566

Closed pixelzoom closed 1 year ago

pixelzoom commented 4 years ago

This work was pulled out of #555.

Over in https://github.com/phetsims/sun/issues/555#issuecomment-585958226:

  • We could rename VerticalAquaRadioButtonGroup to AquaRadioButtonGroup and add an 'orientation' option, like in RadioButtonGroup. That way a few more cases above could be converted.

... and this is in fact needed to be able to "chip away" at sims in #555. So I'll take a stab at it.

pixelzoom commented 4 years ago

So that I can address some of the horizontal layouts, I took a baby step:

RadioButtonGroup has no such convenience classes. But I decided to create some here, mainly so that I didn't need to change the many existing occurrences of VerticalAquaRadioButtonGroup.

pixelzoom commented 4 years ago

Ah... I still need to address these 2 options in AquaRadioButtonGroup, they are specific to vertical orientation:

// dilation of pointer areas for each radio button, y dimension is computed
touchAreaXDilation: 0,
mouseAreaXDilation: 0,
pixelzoom commented 4 years ago

touchArea and mouseArea are now addressed in AquaRadioButtonGroup:

        // dilation of pointer areas for each radio button, perpendicular to options.orientation
        touchAreaDilation: 0,
        mouseAreaDilation: 0,
...
      // See https://github.com/phetsims/sun/issues/555
      assert && assert( options.touchAreaXDilation === undefined, 'touchAreaXDilation is deprecated, use touchAreaDilation' );
      assert && assert( options.mouseAreaXDilation === undefined, 'mouseAreaXDilation is deprecated, use mouseAreaDilation' );
pixelzoom commented 4 years ago

Related to AquaRadioButtonGroup change, Slack discussion with @jessegreenberg:

Chris Malley 4:11 PM VerticalAquaRadioButtonGroup was renamed to AquaRadioButtonGroup. I see this at line 94, option passed to each AquaRadioButton that is created:

a11yNameAttribute: CLASS_NAME + instanceCount

.... where CLASSNAME = 'AquaRadioButtonGroup' . Assuming that is correct, is there anything else that needs to be changed related to this for a11y?

Jesse Greenberg 4:13 PM Thanks! Nope, I don't see anything else that would need to change for a11y

pixelzoom commented 4 years ago

But wait! There's more to do in AquaRadioButtonGroup :). There's a pesky HStrut that shouldn't be there for horizontal orientation.

pixelzoom commented 4 years ago

I didn't like the uniform pointer areas that are set by VerticalRadioButtonGroup:

screenshot_123

And I also don't like how the same strategy gets applied to AquaRadioButtonGroup with horizontal orientation. Here are the origin pointer areas in balancing-chemical equations:

screenshot_125

Here are pointer areas after conversion to HorizontalAquaRadioButtonGroup, which I don't feel is superior, and may in fact be inferior in some cases:

screenshot_126

As I recall, @samreid was the advocate of this "uniform pointer areas strategy", so I'll assign to him for discussion.

Q1: Why is the above VerticalRadioButtonGroup acceptable/preferrable? Q2: What do you recommend for HorizontalAquaRadioButtonGroup?

samreid commented 4 years ago

I think it's good that there is at least some separation between the radio buttons pointer areas in the first balancing-chemical equations example. Maybe the same should be true for vertical radio buttons? I'm not sure about that.

I feel like the uniform pointer areas are easier to use--the "ragged-edge" pointer areas are frustrating to me, and I suspect would be frustrating to some of our users.

pixelzoom commented 4 years ago

I feel like the uniform pointer areas are easier to use--the "ragged-edge" pointer areas are frustrating to me, and I suspect would be frustrating to some of our users.

I don't understand this argument in general, and especially not for the case that I asked about in https://github.com/phetsims/sun/issues/566#issuecomment-586033710, repeated here:

screenshot_123

If the pointer areas were expanded as in the screenshot below... Why would this be frustrating to you? In the screenshot above, why should someone expect to be able to click in a location that is so extremely right-displaced from the labels 'x' and 'y'? And why is that even desirable? It seems like an opportunity to accidentally click somewhere and wonder what happened.

screenshot_128

Uniform pointer areas is a nice feature to have. But I don't think it's necessarily the default behavior, and certainly not appropriate as the only behavior (as it is now).

@arouinfar @ariel-phet can you weight in here?

pixelzoom commented 4 years ago

So that I don't forget, here are a few things that I want to investigate:

arouinfar commented 4 years ago

@pixelzoom said:

Uniform pointer areas is a nice feature to have. But I don't think it's necessarily the default behavior, and certainly not appropriate as the only behavior (as it is now).

@arouinfar @ariel-phet can you weight in here?

This sounds reasonable to me. I think it would be good to have an option for uniform pointer areas, but I don't think it needs to be the default. i would prefer the non-uniform pointer areas in the example shown in https://github.com/phetsims/sun/issues/566#issuecomment-586402164 to the uniform ones.

ariel-phet commented 4 years ago

@arouinfar @pixelzoom I generally agree that uniform pointer areas would be a nice option, but need not be the default.

However, I do not recall - is this uniform pointer area desirable for keyboard nav? I feel like that was one of the original motivations. However, when I keyboard traverse the options in this twitter menu, they follow a "ragged edge" focus strategy:

Recording #1

I will see if @zepumph can comment, but ragged edge as default, and uniform as an option seems correct to me.

zepumph commented 4 years ago

I have seen @terracoda prefer uniform sizing for keyboard focus, but I'm not sure what the exact rationale is. @terracoda can you comment if there is an a11y reason to expand shorter keyboard nav focus out to the length of the longest element?

My own opinion is that ragged edge feels good to me as a default.

pixelzoom commented 4 years ago

Uniform seems to work OK when radio buttons are inside of a container (panel, or some rectangular area). But PhET radio button are sometimes sitting out by themselves (not in a panel), like the radio buttons in Natural Selection (see below). This is where uniform pointer areas really feels wrong to me as a default, especially if the button labels have significantly different widths. (And because strings are translated, we don't know if they will have significantly different widths.)

screenshot_136
pixelzoom commented 4 years ago

@zepumph said:

I have seen @terracoda prefer uniform sizing for keyboard focus, but I'm not sure what the exact rationale is.

@zepumph can you clarify why keyboard focus is relevant here? Is keyboard focus using pointer areas for sizing? If so, is it using mouseArea or touchArea?

terracoda commented 4 years ago

Having large touch areas is generally a good thing to have for accessibility. People using small devices or people with tremors have more success with larger target areas.

The keyboard highlight is relevant because it also shows up on the screen with touch activation on iOS devices when using VoiceOver.

@pixelzoom, touch and keyboard are both input mechanisms. Touch with a screen reader is very much like a keyboard experience because you don't have to be touching the think that you are activating. You just have to visually show focus and clearly communicate where the user's focus is.

I feel we had this conversation along time ago for keyboard highlights and the consensus was to equalize the size of the highlights. I don't see how it doesn't make sense to make the touch area the same size as the keyboard highlight. People often tap next to a label and not right on top of it.

On another note: We won't always get it right the first time, but I feel our goal is to design inclusive components. There is a nice blog (and book about this) by Heydon Pickering. https://inclusive-components.design/

I read Heydon's last blog/book, Inclusive Design Patterns a few years back. I haven't read this new one yet, but I see there are several components that are used in PhET Sims. The HTML/ARIA patterns he codes out are worthy of a deeper investigation.

terracoda commented 4 years ago

I have designed myself into traps here and there when I just thought about keyboard navigation and not about all the input options together.

terracoda commented 4 years ago

People use different fingers for tapping and people have different size fingers. A longer label could support one-hand use and tapping with the thumb.

The article article on touch targets might be helpful https://www.smashingmagazine.com/2012/02/finger-friendly-design-ideal-mobile-touchscreen-target-sizes/

terracoda commented 4 years ago

@ariel-phet example (in https://github.com/phetsims/sun/issues/566#issuecomment-588530255) is a menu - styled with button-like looking things. It is not a group of radio buttons and I do not feel it is a relevant visual comparison.

samreid commented 4 years ago

Twitter's "ragged edge" UI looks nice, but I noticed if you press the "more..." button at the bottom, it shows a popup with the "uniform edge" strategy:

Kapture 2020-02-20 at 8 23 28

So it seems Twitter settled on a combination of both.

@ariel-phet example is a menu - styled with button-like looking things. It is not a group of radio buttos and I do not feel it is a relevant visual comparison.

Good point!

terracoda commented 4 years ago

Nice addition @samreid.

samreid commented 4 years ago

One more point on the twitter example. Even though their displayed highlights are ragged, their touch regions are uniform. This screencast shows me using the mouse to click outside of the area of the visible buttons:

Kapture 2020-02-20 at 8 31 21

terracoda commented 4 years ago

I have also read that radio buttons on mobile are often coded more like buttons, i.e., larger target areas.

I would recommend a larger target area as the default with an option to reduce it to a minimum no smaller than the minimum recommendation. And there is likely little harm in having a PhET minimum larger than the industry standard minimum.

Similar to skateboards, bigger provides easier, smoother experience.

terracoda commented 4 years ago

@samreid, thanks for pointing out the touch targets!

pixelzoom commented 4 years ago

@terracoda said:

Having large touch areas is generally a good thing to have for accessibility.

Agreed! (And I'll assume that you mean all pointer areas, not just touch areas.) But why does that mean that all radio buttons in a group must have the same pointer area size? Can't each radio button just have a sufficiently large size, tailored to its size?

People using small devices or people with tremors have more success with larger target areas.

Agreed - but to a point. If the pointer areas are excessively large, as in the example we've been discussing below, isn't it more likely that they may accidentally choose something with an excessively large pointer area (e.g 'x' and 'y')?

screenshot_123

.... I would recommend a larger target area as the default with an option to reduce it to a minimum no smaller than the minimum recommendation. And there is likely little harm in having a PhET minimum larger than the industry standard minimum.

To clarify... What we're discussing here is whether all of the radio buttons in a group should have the same pointer area size (the size that fits the largest button), not how much that uniform size should be adjusted. So do you recommend an option to allow different pointer areas for each radio button in a group, so that the above example could be adjust to look like this? (The fact that each radio button has a different pointer area size is what's relevant in this example, not the specific size of each pointer area - sizes could be adjusted individually.)

screenshot_128

@ariel-phet example (in #566 (comment)) is a menu - styled with button-like looking things. It is not a group of radio buttons and I do not feel it is a relevant visual comparison.

I disagree. They are both a collection of choices. A radio button is a group of radio buttons. A menu is a group of menu items (typically push buttons). And the means of navigating and choosing is almost identical.

Nice addition @samreid.

I'm confused. @ariel-phet and @samreid's examples are both menus. Please clarify why the former is "not relevant" and the latter is a "nice addition".

pixelzoom commented 4 years ago

I asked:

I'm confused. @ariel-phet and @samreid's examples are both menus. Please clarify why the former is "not relevant" and the latter is a "nice addition".

Ah, I see. @samreid's example is a group of buttons, and pressing one of them results in a menu. So I agree - nice addition. But I still disagree that @ariel-phet's menu example is not relevant.

terracoda commented 4 years ago

@pixelzoom, I meant make them all as big as the biggest one.

pixelzoom commented 4 years ago

I'm not hearing any interest in being able to set pointer areas on a per-button basis. So I'm going to punt on any such option, as well as the things that I was going to investigate in https://github.com/phetsims/sun/issues/566#issuecomment-586403658.

@ariel-phet please assign someone to review. And if there are additional features to be added here, please assign someone else, because I've exceed the time that I can spend on this.

ariel-phet commented 4 years ago

@jessegreenberg is on the review

jessegreenberg commented 1 year ago

Reviewing the work done in this issue:

The rest of the commits changed the pointer area dilation and API for AquaRadioButtonGroup. But after team discussion in this issue those commits were reverted.

AquaRadioButtonGroup and its vertical/horizontal convenience classes look good and have been working well in sims for a long time. This can be closed.