Closed jbphet closed 5 months ago
I talked with @arouinfar a couple days ago about making the responsive description for the flux meter behave like that of the energy balance indicator, which is based on a combination of time and changes to the detected values. I suggested this because I thought we could leverage the existing code for the energy balance indicator. She was up for having us try this.
While working on this today I made a couple of decisions that I'd like to review with @arouinfar. I'll plan to set up a meeting tomorrow. The decisions were:
@arouinfar, @jonathanolson, and I met this morn and discussed the two questions above and the team is good with what I've done for both of them.
As part of today's discussion we also decided to distinguish between "no" and "negligible" flux changes when moving the flux meter, since there are currently cases where greenhouse gases are present and the sensor is being moved but the description is saying "no change in energy flux", which isn't physically accurate.
@arouinfar - I think this feature is to a point where it is ready for you to review. Please check out the behavior on main and let us know what revisions are needed.
@arouinfar and I met to discuss the results of her review. Everything seemed good with one exception: she found it unclear as to when the description would say "no change in energy flux" versus "negligible change in energy flux". I explained that this is probably due to the flux sensor accumulating small changes during the 4-second announcement update window, and could vary based on exactly when in the time window the sensor was dropped by the user. I floated the idea of having the flux meter make an announcement whenever the sensor was dropped and comparing the flux levels at the beginning and end of the drag operation, and resetting the announcement time accumulator for all periodic announcements at that time. @arouinfar said she thought we should give that a try and she'd review it once it was working.
@arouinfar - I've made the changes discussed above. Please review the behavior on main and let us know what you think.
I've tested the flux meter on both the Photons and Layer Model screens in VoiceOver/Safari, and everything matches the spec.
However, @Matthew-Moore240 reported this issue with NVDA on Windows/Chrome.
The second issue is when using the flux meter around the cloud I got double the altitude notifications so it would say "Moderate altitude just above cloud, moderate altitude". It only happens when the sim is playing and I think it's because it is firing both the altitude slider information and giving you the update on what's happening with the sim. It just sounds a bit odd to me.
https://github.com/phetsims/greenhouse-effect/assets/8419308/6087a3fe-8d9f-4722-a988-42e2fe4d1a5f
Thanks for the video recording, that was helpful!
think it's because it is firing both the altitude slider information and giving you the update on what's happening with the sim
I think that is right. When you combine the aria-valuetext with the following alert it sounds like this:
"at low altitude below cloud " "at low altitude, moderate incoming sunlight and very low outgoing sunlight. moderate outgoing infrared and extremely low incoming infrared."
What would you like it to say in this case?
We could change the responsive description to "at flux sensor, moderate incoming sunlight and very low outgoing sunlight. moderate outgoing infrared and extremely low incoming infrared."
Or we could remove the "at low altitude" part after the user has just moved the flux sensor.
Or we could do something else? Or leave as-is? What do you think @arouinfar @Matthew-Moore240?
Thanks for the clarification @jessegreenberg. This is indeed the specified behavior. I don't want to complicate things by adding special-case description. Starting with "At flux sensor..." makes sense if the alert is triggered because the sensor was moved, but it becomes less useful if the sim is alerting because there was a change in flux as the system is equilibrating. Let's leave things as-is, closing.
The parent issue for this is #368.
The flux meter needs to have accessible description added for it. @jessegreenberg, @jonathanolson, and I started working on this and added the basic description for the checkbox that controls the visibility of the flux meter. This went fine for the simpler portions, but some questions came up about how and when the readings in the flux meter should be described. We met with @arouinfar over Zoom and made some modifications to the spec. Long story short, here are the next steps: