-
Notifications
You must be signed in to change notification settings - Fork 7
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
pointer areas sign-off #192
Comments
@amanda-phet this is ready for review. The screenshot below shows pointer areas for the Lab screen, Intro screen is identical. Not shown are the Proportions and Pedigree graphs. Note that the checkboxes in the Population panel do not have uniform width. I'll make them uniform if you want. But unlike radio buttons, there's no support in common code. And imo, uniform width is not desirable for checkboxes or radio buttons. |
@ariel-phet and I discussed the Population panel. I'll make all of the checkboxes the same width. It'll probably work out better for focus traversal. |
Population checkbox touch areas have been expanded, see screenshot below. Proportions checkbox has not - there's only one, so nothing to expand to match. I'd have to do something evil, like add an invisible horizontal strut, to make that one fill the control panel. Pedigree checkboxes are definitely not appropriate for pointer areas that fill the panel. @ariel-phet please review in master. |
Hold off on reviewing. Making the Population checkboxes all have the same size created a new problem, #207. So I'm going to have to come up with a different way of doing this. |
Fixed in the above commit. You may resume reviewing. |
@amanda-phet it looks like @ariel-phet has gotten busy, so assigning to you too. Whichever one of you gets to this first... |
I think it all looks fine. I can't see the pointer areas for the Fur, Ears, and Teeth checkboxes on the Proportions screen. When I try to discern them with my cursor, they don't seem as roomy as the other checkboxes. It seems like I need to be right on the text or box, and not in a roomy rectangle around the text and box. @pixelzoom any way to confirm what those areas are, or expand them slightly? |
Good catch, addressed in the above commit. I've used the same dilation that's used for other checkboxes. See screenshot below. Back to @amanda-phet for review. |
Looks good. Thanks! |
Reopening. After testing on his phone, @ariel-phet has some requests. Increase touchArea for these UI components: Definitely:
Bonus:
We're not going to do these for the prototype RC. |
Here's a patch for the fast-forward button. We thought that 6 looked like a good dilation. But if we want the touchArea to be the same as the play button (and to be computed, instead of a magic number), then the correct value is 4. I'll discuss this with @ariel-phet later. patchIndex: js/common/view/NaturalSelectionTimeControlNode.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- js/common/view/NaturalSelectionTimeControlNode.js (revision 23f61b80837844e5347b912447f78e8ec6439491)
+++ js/common/view/NaturalSelectionTimeControlNode.js (date 1598644626382)
@@ -19,6 +19,10 @@
import naturalSelection from '../../naturalSelection.js';
import FastForwardButton from './FastForwardButton.js';
+// constants
+const PLAY_BUTTON_RADIUS = 20;
+const FAST_FORWARD_BUTTON_RADIUS = 16;
+
class NaturalSelectionTimeControlNode extends HBox {
/**
@@ -40,12 +44,14 @@
}, options );
const playPauseButton = new PlayPauseButton( isPlayingProperty, {
- radius: 20,
+ radius: PLAY_BUTTON_RADIUS,
listener: () => fastForwardButton.interruptSubtreeInput(),
tandem: options.tandem.createTandem( 'playPauseButton' )
} );
const fastForwardButton = new FastForwardButton( timeSpeedProperty, {
+ radius: FAST_FORWARD_BUTTON_RADIUS,
+ touchAreaDilation: PLAY_BUTTON_RADIUS - FAST_FORWARD_BUTTON_RADIUS,
tandem: options.tandem.createTandem( 'fastForwardButton' )
} ); |
I've noticed that for the sim-specific dialogs, the close button feels a little finicky for me on iPad. Since there are no UI components in these dialogs, there's no reason that we can't make their close buttons have generous pointer areas. So I've done that, they are all the same size, and they look like this: |
All requests have been completed. Most of them are shown in the first screenshot below. Generations below the Proportions graph is shown in the second screenshot. See also the screenshots in the previous comment related to Dialog close buttons. @ariel-phet please verify in master, and feel free to close. |
Played on my phone and the few previous problem areas were much more usable. Closing. |
This is listed as a "standard GitHub issue" in the code-review checklist. The lead designer must sign off on pointer areas.
The text was updated successfully, but these errors were encountered: