phetsims / natural-selection

"Natural Selection" is an educational simulation in HTML5, by PhET Interactive Simulations
GNU General Public License v3.0
3 stars 7 forks source link

Hidden `navigationBar.titleText` doesn't migrate from 1.4 to 1.5 #352

Closed KatieWoe closed 1 year ago

KatieWoe commented 1 year ago

Test device Samsung Operating System Win 11 Browser Chrome Problem description For https://github.com/phetsims/qa/issues/967 If you hide the title text in 1.4 with naturalSelection.general.view.navigationBar.titleText.visibleProperty and then migrate, the title text is still there in 1.5.

Visuals notmigrated

Troubleshooting information:

!!!!! DO NOT EDIT !!!!! Name: ‪Natural Selection‬ URL: https://phet-dev.colorado.edu/html/natural-selection/1.5.0-dev.5/phet/natural-selection_all_phet.html Version: 1.5.0-dev.5 2023-07-31 18:14:35 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/115.0.0.0 Safari/537.36 Language: en-US Window: 1536x707 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: 31 uniform: 4096 Texture: size: 8192 imageUnits: 32 (vertex: 32, combined: 64) Max viewport: 8192x8192 OES_texture_float: true Dependencies JSON: {}
pixelzoom commented 1 year ago

navigationBar.titleText.visibleProperty is no longer instrumented. So natural-selection-migration-rules.ts (and other sims) contain this rule:

    // These elements in the navigationBar no longer exist because we do not automatically instrument visibleProperty for Text and RichText.
    // See https://github.com/phetsims/scenery/issues/1447
    new TandemFragmentDelete( `${simName}.general.view.navigationBar.titleText.visibleProperty` ),

This was a common-code change related to https://github.com/phetsims/scenery/issues/1447. As is, I don't see how we can do anything other ignore it via TandemFragmentDelete. But it does prevent the client from being able to hide the title in the navigation bar, and I don't know if that was a conscious decision or an accidental side-effect.

Assigning to @arouinfar and @zepumph for comment, and to decide whether anything needs to be done here.

arouinfar commented 1 year ago

But it does prevent the client from being able to hide the title in the navigation bar, and I don't know if that was a conscious decision or an accidental side-effect.

I can't remember if this was explicitly discussed, but I am okay with this behavior. I don't think we should be making it easy for clients to hide the sim name. It's still possible if you set enough StringProperties to an empty string, so a motivated client could still do it if they wanted to.

pixelzoom commented 1 year ago

Closing as "won't fix".