phetsims / atomic-interactions

"Atomic Interactions" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
1 stars 2 forks source link

Pinned and Moving strings very small #74

Closed KatieWoe closed 2 years ago

KatieWoe commented 6 years ago

Test device: Dell Laptop Operating System: Win 10 Browser: chrome Problem description: For https://github.com/phetsims/QA/issues/216 When stringTest=double, some of the strings become small rather quickly (custom attraction in particular) and Pinned and Moving seem to become quite tiny. This seems to be in part due to an apparent change in size and position of the toolbox between versions. These strings were smaller in the published version, but it seems more prominent now. Steps to reproduce:

  1. Set stringTest=double
  2. Set stringTest=double on the published version
  3. Compare

Screenshots: The gif bellow shows the rc on the left tab, and the published version on the right size

Troubleshooting information (do not edit):

Name: ‪Atomic Interactions‬:‪Atomic Interactions‬ URL: https://phet-dev.colorado.edu/html/atomic-interactions/1.1.0-rc.1/phet/atomic-interactions_en_phet.html?stringTest=double Version: 1.1.0-rc.1 2018-11-05 21:18:48 UTC Features missing: touch Flags: pixelRatioScaling User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/70.0.3538.110 Safari/537.36 Language: en-US Window: 1536x732 Pixel Ratio: 2.5/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: 16384x16384 OES_texture_float: true Dependencies JSON: {"assert":{"sha":"928741cf","branch":"HEAD"},"atomic-interactions":{"sha":"15bb3a2d","branch":"HEAD"},"axon":{"sha":"d7ba44e2","branch":"HEAD"},"brand":{"sha":"89d28f63","branch":"HEAD"},"chipper":{"sha":"bbbb514a","branch":"HEAD"},"dot":{"sha":"820111c8","branch":"HEAD"},"joist":{"sha":"749f10ff","branch":"HEAD"},"kite":{"sha":"59755f7e","branch":"HEAD"},"nitroglycerin":{"sha":"ccaab9cd","branch":"HEAD"},"phet-core":{"sha":"68a89cc4","branch":"HEAD"},"phet-io":{"sha":"364077cf","branch":"HEAD"},"phet-io-wrapper-classroom-activity":{"sha":"abc4ae3e","branch":"HEAD"},"phet-io-wrapper-hookes-law-energy":{"sha":"429886e7","branch":"HEAD"},"phet-io-wrapper-lab-book":{"sha":"ef0b0b0c","branch":"HEAD"},"phet-io-wrappers":{"sha":"7ec1a04a","branch":"HEAD"},"phetcommon":{"sha":"22b158d0","branch":"HEAD"},"query-string-machine":{"sha":"c2e502fb","branch":"HEAD"},"scenery":{"sha":"415371c3","branch":"HEAD"},"scenery-phet":{"sha":"94b92879","branch":"HEAD"},"sherpa":{"sha":"a9fd066b","branch":"HEAD"},"states-of-matter":{"sha":"d6317e0b","branch":"HEAD"},"sun":{"sha":"3086e97a","branch":"HEAD"},"tambo":{"sha":"0e5b9133","branch":"HEAD"},"tandem":{"sha":"a2348c0c","branch":"HEAD"}}
arouinfar commented 6 years ago

@jbphet looks like the panel in the rc is quite a bit narrower than the published version. image

The maxWidth of Pinned, Moving, and Custom Attraction all seem a bit smaller than necessary image

Similarly, the maxWidth of the strings in the Forces panel all look a bit small too. I think the maxWidth could extend to the magenta line. image

KatieWoe commented 6 years ago

I've seen that on lower resolution screens (Chrome OS - Lovelace in this case) the small strings in the image above (van der Walls and electron overlap) are not legible. Since these max widths will likely be lengthened this may be less of a problem. What do you think @arouinfar?

arouinfar commented 6 years ago

@KatieWoe can you take a screenshot on Lovelace?

I won't make any recommendations until the maxWidth issue has been addressed.

KatieWoe commented 6 years ago

pastedimage 1 I've also noted that the arrows move to the left when the strings are short. Is that intended? pastedimage

arouinfar commented 6 years ago

@KatieWoe good observation. I believe the position of the arrows is determined by the end of the longest string, so they will shift with varying string width/font size. So long as the alignment is maintained, I don't think that's an issue.

The issue here is that the maxWidths are just too small, and should be made as large as possible while maintaining some reasonable amount of padding.

jbphet commented 6 years ago

This sim is going on the back burner for a while due to other priorities. Unassigning, will reassign when work resumes.

SaurabhTotey commented 5 years ago

I have tried increasing the max widths in as many ways as I could. I still don't notice a huge difference after my changes, but I do think the differences in widths are a little noticeable. I will leave a status:ready-for-review label on this issue so that the current changes can be reviewed and I can be later instructed on what to do next.

SaurabhTotey commented 5 years ago

I went back through all the width variables that I changed in https://github.com/phetsims/states-of-matter/commit/dd511c8c054a09213f34c414030f02feee9f9211 in a more thorough manner. After a more careful examination, I was able to find the variables that actually changed what I was looking to change. I then steepened those values so that the max widths of the relevant pieces of the panels are better. However, I think it still might be a good idea to widen and heighten the panels just a little bit.

SaurabhTotey commented 5 years ago

I have now made my final touches. I am unsure of what the end goal is for these panels, but I have tried increasing the max width for the text in these panels for atomic-interactions while not affecting states-of-matter too heavily. Assigning to @arouinfar to get feedback and/or further advice.

SaurabhTotey commented 5 years ago

Removing status:ready-for-review label as this issue is assigned to @arouinfar.

arouinfar commented 5 years ago

@SaurabhTotey the maxWidths are all looking good in master. I think it would be helpful to make the panel a bit wider still (and scale up the maxWidths accordingly). Let's try a 10% increase in panel width.

SaurabhTotey commented 5 years ago

In master the panel width has increased from 190 to 209. Here is what the change looks like.

Old:

190

New:

209

The change is slight, so I can widen the panels even more if necessary.

arouinfar commented 5 years ago

@SaurabhTotey the updated panel width is looking nice. One thing I've noticed in master is that there's quite a bit of extra padding to the left of the radio buttons, which isn't there in your screenshots.

image

SaurabhTotey commented 5 years ago

I didn't even notice that! It seems like the problem arises because the button-group is center-aligned rather than left-aligned. This wasn't visible before because the buttons used to always take up the full space. However, after widening the panel, I guess this is now more visible. I changed the alignment in my latest commit so that they are now left-aligned again and there doesn't seem to be as much padding. The radio buttons from the top control panel now again align with the radio buttons from the forces panel. The padding right now is set to 10 pixels, but I can reduce it further if needed.

control_panel
arouinfar commented 5 years ago

Thanks @SaurabhTotey, looks good!

KatieWoe commented 5 years ago

String lengths themselves look ok

jbphet commented 4 years ago

I ran into this when doing phet-io instrumentation of SOM, see https://github.com/phetsims/states-of-matter/issues/266. The panels now seem to have an odd gap on the right side in both SOM and AI, and it doesn't look good to my eye. @SaurabhTotey - can you please revisit this and make the panels look more balanced? Even when I use the long string tests, this space is still present on the side.

Screenshots from the current master versions:

image

image

image

SaurabhTotey commented 4 years ago

Here is what the panels look like now in SOM and AI respectively. som ai

Assigning to @arouinfar to determine what more should be done, if anything. @jbphet suggested that it might look nice to align the forces arrows with the atoms in the atoms panel.

arouinfar commented 2 years ago

This was addressed in the 1.2 release and it looks fine to me, closing.