phetsims / sherpa

Third-party libraries and dependencies for PhET Simulations
http://scenerystack.org/
Other
8 stars 8 forks source link

Should we use checkbox icons from Font Awesome 4 or 5? #84

Closed samreid closed 3 years ago

samreid commented 3 years ago

From https://github.com/phetsims/expression-exchange/issues/154 and https://github.com/phetsims/sherpa/issues/83, we have mostly upgraded to Font Awesome 5, but one of the last Font Awesome 4 icons we are using is the checkbox. Our general consensus is that we have generally liked the style, simplicity and consistency of using font awesome 5 for everything (though some of us prefer the Font Awesome 4 scissors). However, we are still using this font awesome 4 icon for the checkboxes:

https://fontawesome.com/v4.7/icon/check-square-o

We custom-made an empty checkbox based on that style.

Font awesome 5 provides these checkboxes:

https://fontawesome.com/v5.15/icons/check-square?style=regular https://fontawesome.com/v5.15/icons/check-square?style=solid

https://fontawesome.com/v5.15/icons/square?style=regular

The easiest way of upgrading in master would leave a period of time where some sims have the new style and some sims have the old style.

@arouinfar said:

I didn't realize checkboxes were also FA. This is a much bigger discussion that needs to involve @kathy-phet and all relevant stakeholders, perhaps at a design meeting.

@amanda-phet said:

I agree with Amy that we need to have a larger discussion about checkboxes since those would be a major change. Since the checkbox design existed before we were using font awesome, I wasn't aware we were using font awesome for checkboxes.

samreid commented 3 years ago

Here's a patch that uses the FA5 checkboxes:

```diff Index: js/Checkbox.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/Checkbox.js b/js/Checkbox.js --- a/js/Checkbox.js (revision 90def436590179e00efd80f6d55bffcd3137db2a) +++ b/js/Checkbox.js (date 1626727566761) @@ -19,8 +19,9 @@ import Path from '../../scenery/js/nodes/Path.js'; import Rectangle from '../../scenery/js/nodes/Rectangle.js'; import SceneryConstants from '../../scenery/js/SceneryConstants.js'; -import checkEmptySolidShape from '../../sherpa/js/fontawesome-4/checkEmptySolidShape.js'; -import checkSquareOSolidShape from '../../sherpa/js/fontawesome-4/checkSquareOSolidShape.js'; +import squareSolidShape from '../../sherpa/js/fontawesome-5/squareSolidShape.js'; +import squareRegularShape from '../../sherpa/js/fontawesome-5/squareRegularShape.js'; +import checkSquareRegularShape from '../../sherpa/js/fontawesome-5/checkSquareRegularShape.js'; import checkboxCheckedSoundPlayer from '../../tambo/js/shared-sound-players/checkboxCheckedSoundPlayer.js'; import checkboxUncheckedSoundPlayer from '../../tambo/js/shared-sound-players/checkboxUncheckedSoundPlayer.js'; import EventType from '../../tandem/js/EventType.js'; @@ -30,9 +31,10 @@ // constants const BOOLEAN_VALIDATOR = { valueType: 'boolean' }; -const SHAPE_MATRIX = Matrix3.createFromPool( 0.025, 0, 0, 0, -0.025, 0, 0, 0, 1 ); // to create a unity-scale icon -const uncheckedShape = checkEmptySolidShape.transformed( SHAPE_MATRIX ); -const checkedShape = checkSquareOSolidShape.transformed( SHAPE_MATRIX ); +const SHAPE_MATRIX = Matrix3.createFromPool( 0.025, 0, 0, 0, 0.025, 0, 0, 0, 1 ); // to create a unity-scale icon +const uncheckedShape = squareRegularShape.transformed( SHAPE_MATRIX ); +const checkedShape = checkSquareRegularShape.transformed( SHAPE_MATRIX ); +const backgroundShape = squareSolidShape.transformed( SHAPE_MATRIX ); class Checkbox extends Node { @@ -102,10 +104,10 @@ // @private - Create the background. Until we are creating our own shapes, just put a rectangle behind the font // awesome checkbox icons. - this.backgroundNode = new Rectangle( 0, -options.boxWidth, options.boxWidth * 0.95, options.boxWidth * 0.95, - options.boxWidth * 0.2, options.boxWidth * 0.2, { - fill: options.checkboxColorBackground - } ); + // this.backgroundNode = new Rectangle( 0, -options.boxWidth, options.boxWidth * 0.95, options.boxWidth * 0.95, + // options.boxWidth * 0.2, options.boxWidth * 0.2, { + // fill: options.checkboxColorBackground + // } ); // @private this.uncheckedNode = new Path( uncheckedShape, { @@ -120,6 +122,10 @@ fill: options.checkboxColor } ); + this.backgroundNode = new Path( backgroundShape, { + fill: options.checkboxColorBackground + } ); + // @private this.checkboxNode = new Node( { children: [ this.backgroundNode, this.checkedNode, this.uncheckedNode ] } ); ```

image

arouinfar commented 3 years ago

Thanks for the mockup @samreid! I think it'll really help the discussion.

samreid commented 3 years ago

Thanks! I was skeptical about the new one since I like the Font Awesome 4 checkbox icon so much, but in testing the Font Awesome 5 one, it also seemed good.

samreid commented 3 years ago

@amanda-phet: I prefer the FA4 one, it fits well with our style @samreid: The FA5 one is crisper, but the FA4 version "pops" more and is easier to visually parse at a glance.

@amanda-phet asked if we have technical reasons to move fully away from FA4. I pointed out that many of the technical approaches from FA5 (getting rid of FontAwesomeNode, using modular strings, etc) will work equally well for FA4 and it's not a technical problem to keep around a few FA4 shapes. Likewise, we haven't had a problem with the FA4 license and don't expect to, so it seems good to keep it as it is.

We hear sometimes from other group designers (not users) that the checkboxes look like they are for a younger audience, but since it seems clear and visually parse-able, we are happy with keeping it as it is.

We agreed to keep it as it is, closing.