-
Notifications
You must be signed in to change notification settings - Fork 6
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
factor out HomeScreenButton #183
Comments
No plans to work on this in the near future, unassigning. |
To clarify... "Screen buttons" means the buttons for selecting a screen that appear on the home screen and in the navbar. There is some additional discussion in #159. |
To summarize the status of this issue... JoistButton was created for use in the navigation bar, but doesn't seem suited to the Home screen buttons. Home screen buttons are created in HomeScreenView. The very least that we should do is factor out the code that creates the buttons into something like HomeScreenButton, so it's the button code isn't mixed in with other responsibilities of HomeScreenView. This will make it easier to maintain the Home screen buttons. When we get around to this is up to project management. |
Also worth noting is that there are currently 2 buttons (vars |
If/when HomeScreenButton is factored out, set |
The same tandem names can be used, and I don't think any 3rd party is yet leveraging the home screen icon APIs, so it would be OK to change. I'm interested in changing this button implementation because right now the button is a VBox. var largeScreenButton = new VBox( {
//Don't 40 the VBox or it will shift down when the border becomes thicker
resize: false,
cursor: 'pointer',
children: [
largeIconWithFrame,
largeText
],
textDescription: screen.name + ' Screen: Button',
accessibleContent: {
createPeer: function( accessibleInstance ) {
// We want DOM elements to look like this:
// <input type="button" tabIndex="0" value="screenName" id="largeButton-index">
var domElement = document.createElement( 'input' );
domElement.setAttribute( 'type', 'button' );
domElement.setAttribute( 'value', screen.name );
domElement.id = 'largeButton-' + index;
domElement.tabIndex = '0';
// enter the selected screen on 'click'
domElement.addEventListener( 'click', function() {
largeButtonDown();
} );
return new AccessiblePeer( accessibleInstance, domElement );
}
}
} ); If we coalesce the buttons, a different tandem can be assigned--it wouldn't disrupt PhET-iO clients at this point. |
This work was done in #601 |
The screen buttons should be revisited. We should decide if we are going to use a starting point of (a) JoistButton as defined in #182 or (b) the new radio buttons as discussed here: phetsims/sun#127
The text was updated successfully, but these errors were encountered: