search

phetsims / keplers-laws

"Kepler's Laws" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
1 stars 1 forks source link

Mars marker on graph is initially obscured #262

Closed Nancy-Salpepi closed 7 months ago

Nancy-Salpepi commented 7 months ago

Test device MacBook Air M1 chip

Operating System 14.3

Browser Safari 17.3

Problem description For https://github.com/phetsims/qa/issues/1034, the Mars "dot" is initially hidden by the pink dot.

Screenshot 2024-02-12 at 11 32 14 AM
Nancy-Salpepi commented 7 months ago

it looks good when the 2 dots overlap but when they don't, the gray dot seems huge. @arouinfar @AgustinVallejo do you think it looks ok?

Screenshot 2024-02-12 at 1 11 50 PM Screenshot 2024-02-12 at 1 12 18 PM
AgustinVallejo commented 7 months ago

Yup, the straightforward solution was to just enlarge the target orbit dot. @arouinfar let me know if you'd rather only make it big if it's obscured by the pink one.

AgustinVallejo commented 7 months ago

@arouinfar answered in slack to scale up the grey marker only when it's obscured by the other one.

AgustinVallejo commented 7 months ago

Above commit addresses that change, please review and mark for cherry picking :)

Nancy-Salpepi commented 7 months ago

The gray dot get larger as soon as there is any overlap. Just wanted to confirm with @arouinfar that this is working as desired.

https://github.com/phetsims/keplers-laws/assets/87318828/4521becc-d46d-43e0-8f93-3402317d77d6

arouinfar commented 7 months ago

Thanks for checking @Nancy-Salpepi.

@AgustinVallejo I think the behavior in Nancy's video is acceptable, but it would look nicer if the gray dot grows only when there is perfect overlap.

AgustinVallejo commented 7 months ago

Assigning back for review

Nancy-Salpepi commented 7 months ago

Looks much better to me! The gray dot is still enlarged over a range of values but that range is much smaller.

Nancy-Salpepi commented 7 months ago

looks good in rc.2