Skip to content
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

Hard to see photons #252

Closed
Tracked by #885
KatieWoe opened this issue Jan 6, 2023 · 7 comments
Closed
Tracked by #885

Hard to see photons #252

KatieWoe opened this issue Jan 6, 2023 · 7 comments

Comments

@KatieWoe
Copy link
Contributor

KatieWoe commented Jan 6, 2023

Test device
Samsung
Operating System
Win 11
Browser
Chrome
Problem description
For phetsims/qa#871
The yellow color of the photons are hard to see on white or light backgrounds. This can be seen on the third screen in the Sunlight box and on the ground when the albedo is high.

Visuals
hardlight

Troubleshooting information:

!!!!! DO NOT EDIT !!!!!
Name: ‪Greenhouse Effect‬
URL: https://phet-dev.colorado.edu/html/greenhouse-effect/1.1.0-rc.1/phet/greenhouse-effect_all_phet.html
Version: 1.1.0-rc.1 2022-12-23 00:15:29 UTC
Features missing: applicationcache, applicationcache, touch
Flags: pixelRatioScaling
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/108.0.0.0 Safari/537.36
Language: en-US
Window: 1536x714
Pixel Ratio: 1.25/1
WebGL: WebGL 1.0 (OpenGL ES 2.0 Chromium)
GLSL: WebGL GLSL ES 1.0 (OpenGL ES GLSL ES 1.0 Chromium)
Vendor: WebKit (WebKit WebGL)
Vertex: attribs: 16 varying: 30 uniform: 4096
Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 32)
Max viewport: 32767x32767
OES_texture_float: true
Dependencies JSON: {}

@KatieWoe KatieWoe added the type:bug Something isn't working label Jan 6, 2023
@jbphet
Copy link
Contributor

jbphet commented Jan 6, 2023

@arouinfar and I reviewed this and decided to try putting a background behind the photon icons in the panels, so I'll do that. We're thinking that a dark blue that matches part of the upper sky would be a good place to start. As for the contrast with the ground when the albedo is high, this seems to be less of a concern since the photons are generally moving and thus hard to lose sight of.

@samreid
Copy link
Member

samreid commented Jan 9, 2023

A quick mock-up about making the banner color change:

image

@jbphet
Copy link
Contributor

jbphet commented Jan 9, 2023

I tried using a dark blue background, but it didn't work very well due to the transparency in the photons. After some experimentation, I thought a plain black background looked best.

@arouinfar - here's a screenshot. This has not been committed because I wanted your input first. Should we go with this, or something like the "black banner" approach that @samreid mocked up above, leave it as is, or something else?

image

Patch for this change:

Index: js/layer-model/view/SunAndReflectionControl.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/layer-model/view/SunAndReflectionControl.ts b/js/layer-model/view/SunAndReflectionControl.ts
--- a/js/layer-model/view/SunAndReflectionControl.ts	(revision aa29ef119cc258fe7fb165acf200e6b8468bff7a)
+++ b/js/layer-model/view/SunAndReflectionControl.ts	(date 1673303457377)
@@ -11,8 +11,9 @@
 import Range from '../../../../dot/js/Range.js';
 import Utils from '../../../../dot/js/Utils.js';
 import { combineOptions } from '../../../../phet-core/js/optionize.js';
+import BackgroundNode from '../../../../scenery-phet/js/BackgroundNode.js';
 import PhetFont from '../../../../scenery-phet/js/PhetFont.js';
-import { HBox, Image, Text, VBox } from '../../../../scenery/js/imports.js';
+import { Color, HBox, Image, Text, VBox } from '../../../../scenery/js/imports.js';
 import HSlider from '../../../../sun/js/HSlider.js';
 import Panel from '../../../../sun/js/Panel.js';
 import { SliderOptions } from '../../../../sun/js/Slider.js';
@@ -75,10 +76,20 @@
       tandem: options.tandem.createTandem( 'titleText' )
     } );
 
-    // Image of a photon that will be combined with the title text to form the overall title for the panel.
-    const visiblePhotonIcon = new Image( visiblePhoton_png, {
-      maxWidth: 20 // empirically determined to look how we want it
-    } );
+    // Icon of a photon on a background that will be combined with the title text to form the overall title for the
+    // panel.
+    const visiblePhotonIcon = new BackgroundNode(
+      new Image( visiblePhoton_png, {
+        maxWidth: 20 // empirically determined to look how we want it
+      } ),
+      {
+        rectangleOptions: {
+          fill: Color.BLACK,
+          opacity: 1,
+          cornerRadius: 2
+        }
+      }
+    );
 
     const titleNode = new HBox( {
       children: [ titleText, visiblePhotonIcon ],
Index: js/layer-model/view/LayersControl.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/layer-model/view/LayersControl.ts b/js/layer-model/view/LayersControl.ts
--- a/js/layer-model/view/LayersControl.ts	(revision aa29ef119cc258fe7fb165acf200e6b8468bff7a)
+++ b/js/layer-model/view/LayersControl.ts	(date 1673303457376)
@@ -11,8 +11,9 @@
 import Dimension2 from '../../../../dot/js/Dimension2.js';
 import Range from '../../../../dot/js/Range.js';
 import Utils from '../../../../dot/js/Utils.js';
+import BackgroundNode from '../../../../scenery-phet/js/BackgroundNode.js';
 import PhetFont from '../../../../scenery-phet/js/PhetFont.js';
-import { HBox, Image, Text, VBox } from '../../../../scenery/js/imports.js';
+import { Color, HBox, Image, Text, VBox } from '../../../../scenery/js/imports.js';
 import HSlider from '../../../../sun/js/HSlider.js';
 import NumberPicker from '../../../../sun/js/NumberPicker.js';
 import Panel from '../../../../sun/js/Panel.js';
@@ -66,10 +67,20 @@
       tandem: options.tandem.createTandem( 'titleText' )
     } );
 
-    // image of a photon that will be combined with the title text to form the overall title for the panel
-    const infraredPhotonIcon = new Image( infraredPhoton_png, {
-      maxWidth: 20 // empirically determined to look how we want it
-    } );
+    // Icon of a photon on a background that will be combined with the title text to form the overall title for the
+    // panel.
+    const infraredPhotonIcon = new BackgroundNode(
+      new Image( infraredPhoton_png, {
+        maxWidth: 20 // empirically determined to look how we want it
+      } ),
+      {
+        rectangleOptions: {
+          fill: Color.BLACK,
+          opacity: 1,
+          cornerRadius: 2
+        }
+      }
+    );
 
     const titleNode = new HBox( {
       children: [ titleText, infraredPhotonIcon ],

@jbphet jbphet assigned arouinfar and unassigned jbphet Jan 9, 2023
@arouinfar
Copy link
Contributor

Thanks for the mockups @samreid. It was really helpful to see both the banner and icon background options.

I think the black square behind the icon is less obtrusive and better integrated than the extended banner, but I don't really like this direction. It emphasizes the icons in a distracting way and it repeats the information that is in the legend above.

@jbphet and I decided to remove the icons from the Sunlight/Infrared panels, so back to @jbphet.

@arouinfar arouinfar assigned jbphet and unassigned arouinfar Jan 11, 2023
@arouinfar arouinfar added design:polish and removed type:bug Something isn't working labels Jan 11, 2023
jbphet added a commit that referenced this issue Jan 13, 2023
jbphet added a commit that referenced this issue Jan 13, 2023
(cherry picked from commit 555c14a)
@jbphet
Copy link
Contributor

jbphet commented Jan 13, 2023

The photons have been removed from the panels on both the 1.1 and master branches.

@jbphet
Copy link
Contributor

jbphet commented Jan 18, 2023

Note to QA testers: The response to this issue was to remove the photons from two panels and otherwise leave the behavior as is, so that's what will need to be verified.

@Nancy-Salpepi
Copy link

Icons have been removed in rc.2. Closing.

Screenshot 2023-01-20 at 3 31 59 PM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants