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

Stopwatch always in front even if voltmeter is grabbed #280

Closed
brooklynlash opened this issue Jan 13, 2021 · 13 comments
Closed

Stopwatch always in front even if voltmeter is grabbed #280

brooklynlash opened this issue Jan 13, 2021 · 13 comments
Assignees
Labels

Comments

@brooklynlash
Copy link

brooklynlash commented Jan 13, 2021

Test device
Lenovo ThinkPad

Operating System
Windows10

Browser
Chrome, Firefox

Problem description
For phetsims/qa#588
In the "Light" section, both the stopwatch and voltmeter can be grabbed, but even if you grab and drag around the voltmeter, it will not pop to the front.

Visuals
capacitorstopwatch

Troubleshooting information:
!!!!! DO NOT EDIT !!!!!
Name: ‪Capacitor Lab: Basics‬
URL: https://phet-dev.colorado.edu/html/capacitor-lab-basics/1.7.0-rc.8/phet/capacitor-lab-basics_all_phet.html
Version: 1.7.0-rc.8 2020-12-17 01:25:13 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/87.0.4280.141 Safari/537.36
Language: en-US
Window: 1536x722
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: 4095
Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 32)
Max viewport: 32767x32767
OES_texture_float: true
Dependencies JSON: {}

@arouinfar
Copy link

arouinfar commented Jan 15, 2021

This looks a bit weird to me @Denz1994, but I think it's a relatively minor issue. If it's easy to do, it would be nice to bring the selected tool to the front. If the z-ordering is more complicated than that, we can close this as wontfix.

@jonathanolson
Copy link
Contributor

Fixed in master with the above commit (can you verify?)

Also the rectangular hit-box for the voltmeter body seemed unfortunate (made the voltmeter a pain, as its hitbox extended well past its visual). I added pixel hit-testing for the body (which works nicely in master, so the hit area matches the visual)... however I used a Scenery flag that was implemented ~7 months ago.

As this unpublished RC version is using SHAs from... closer to 2 years ago, I'm curious about what common code it should be using (since that improvement will be ignored by the older SHAs).

Thoughts?

@jonathanolson
Copy link
Contributor

Also, for this reason I did NOT push this to the 1.7 branch. Please explicitly let me know if I should do so (stacking will work, hit area will be weird).

@arouinfar
Copy link

@jonathanolson this is looking good in master.

As this unpublished RC version is using SHAs from... closer to 2 years ago

Yikes, I knew they were old, but I didn't realize quite how old. This redeploy has been back-burnered a few times because of competing priorities, and I really want to get it over the finish line. That said, it doesn't seem advisable to publish on such a stale branch. Taking new SHAs would require more QA time, but it would save @jonathanolson from cherry-picking the latest batch of sim changes.

My vote would be for taking new SHAs, but I can't make that call. @kathy-phet what would you advise?

@arouinfar arouinfar assigned kathy-phet and unassigned arouinfar Feb 3, 2021
@kathy-phet
Copy link

kathy-phet commented Feb 3, 2021

Yes, let's take new SHA's. Two years is really old ... yikes!
@jonathanolson - Do we need to do a new dev on master, to test before moving it to a new RC with new SHAs?

@jonathanolson
Copy link
Contributor

Do we need to do a new dev on master, to test before moving it to a new RC with new SHAs?

That would be my general preference, given the number of sim-specific code differences I saw between 1.7 and master. I'm also fine starting with an RC for 1.8 so that we don't need to redo any testing.

Preferences @kathy-phet or @arouinfar?

@arouinfar
Copy link

I think it makes sense to start with a dev test, but that means the QA tail will be longer. That's okay with me so long as we don't let things get too stale again.

@arouinfar arouinfar removed their assignment Feb 3, 2021
@kathy-phet
Copy link

Let's start with dev test - once QA load lightens a bit - it will be good and dovetails with the idea that we will hopefully fit PhET-iO upgrade to new studio for this simulation later this year. So dev testing on the current master (phet brand) would be a good first step to be set up for that.

@kathy-phet
Copy link

@jonathanolson - can you please make an new QA issue, but hold off on creating building the dev version until we are ready to put it into the active test queue.

@KatieWoe
Copy link
Contributor

This does look fixed in dev 5. Closing

@brooklynlash
Copy link
Author

brooklynlash commented Mar 31, 2021

Reopening, because when you press the play or reset button for the stopwatch, it still stays behind the voltmeter. Kind of an inverted issue but I think it is similar enough.
capacitance
phetsims/qa#616

@brooklynlash brooklynlash reopened this Mar 31, 2021
@jonathanolson
Copy link
Contributor

Due to the recent work in phetsims/scenery-phet#659, this should be working better now in general. I tested master and it seems fine (moves to the front properly).

@brooklynlash can you test on master (phettest) to confirm?

@brooklynlash
Copy link
Author

Looks fixed now! Thanks again. Re-closing.

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

No branches or pull requests

6 participants