phetsims / fourier-making-waves

"Fourier: Making Waves" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
6 stars 3 forks source link

Measurement tool covers +/- zoom buttons #204

Closed KatieWoe closed 2 years ago

KatieWoe commented 2 years ago

Test device Jordan Operating System MacOS 10.13 Browser Safari Problem description For https://github.com/phetsims/qa/issues/711. The green lambda tool on the third screen will appear in such a way that the +/- buttons for the graph are fully visible, but can't be clicked easily since they are mostly covered in the area between the two end points. This isn't clear and can lead to the impression that the buttons aren't working. The measurement tool is easily moveable, but this is more of a concern if the tool appears like this.

Visuals scalescoverbuttons

pixelzoom commented 2 years ago

By design, measurement tools have pointer areas that are shaped like the solid red outline that I've Photoshopped into this screenshot:

screenshot_1306

This makes it easy to drag the "calipers" part of the tool, and also makes the label draggable. But it also means that things that are visible between the jaws of the calipers will not receive input events. In the case that @KatieWoe has reported, the zoom buttons are between the caliper jaws, therefore behind the tool's pointer area, and therefore the zoom buttons will not receive input event.. More generally, anything that is behind this pointer area will not receive input events.

Also note that these pointer areas are not shown with ?showPointerAreas because they are the default, and scenery does not display default pointer areas.

@arouinfar would you like anything changed here?

pixelzoom commented 2 years ago

Raising priority to high, because QA is done with 1.0.0-rc.1.

arouinfar commented 2 years ago

It probably wouldn't be obvious to users that they are clicking on the caliper tool in @KatieWoe's gif, so the behavior looks buggy. However, I wouldn't change the pointer areas of the caliper tool. I also don't know that there should/could be more sophisticated detection in scenery to choose the UI component on the bottom in situations like these.

I don't have a good solution to offer here, other than to move the default position of the wavelength tool on the Wave Packet screen. Its default position is likely to cause overlap with the zoom buttons. image

@pixelzoom let's move the default position up a bit image

pixelzoom commented 2 years ago

Good idea about moving the initial position of the tool. The current initial position is a holdover from before we had checkboxes, someplease "out of the way". So I like the position shown below even better, because it's definitely the best place to take measurements. So that's what I did (in master and 1.0). @arouinfar please review. I can change it if you don't like it.

screenshot_1311
arouinfar commented 2 years ago

Looks good to me, thanks @pixelzoom!

pixelzoom commented 2 years ago

Ready for testing in the next RC. To verify:

  1. Run the sim
  2. Go to Wave Packet screen
  3. Check the checkbox for the lambda1 tool (green).
  4. Verify that the tool's initial position is as shown in https://github.com/phetsims/fourier-making-waves/issues/204#issuecomment-933941459.
Nancy-Salpepi commented 2 years ago

When Width Indicators is checked, the measurement tool covers the label. I know the tool is moveable, but I thought it was worth mentioning. @arouinfar what do you think?

Screen Shot 2021-10-11 at 2 23 05 PM
arouinfar commented 2 years ago

Thanks for checking @Nancy-Salpepi. Measuring from the origin is most natural, but we tried moving the tool to the bottom of the graph to make it a bit more out of the way, but still useful. However, that position lead to blocking the zoom button's pointer areas without visually occluding them which was problematic. I think the overlap with the width is minor, so it's better to leave things as-is.