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

Clicking on listbox may activate dropper #85

Closed
KatieWoe opened this issue May 30, 2019 · 15 comments
Closed

Clicking on listbox may activate dropper #85

KatieWoe opened this issue May 30, 2019 · 15 comments
Labels

Comments

@KatieWoe
Copy link
Contributor

Test device:
Dell
Operating System:
Win 10 (or other)
Browser:
Chrome (or other)
Problem description:
For phetsims/qa#323 and phetsims/qa#326.
Likely minor. Not in published version. It is possible to activate the dropper button when you are actually clicking on part of the dropdown menu that selects chemicals.
Steps to reproduce:

  1. Set ?showPointerAreas as it makes this easier to do
  2. Leave the dropper in the default position
  3. Open the dropdown to select chemicals
  4. If your mouse is on the far right of the menu and overlaps the blue button, you can click in the menu but instead activate the dropper. If the menu item highlights, this does not happen, so it seems to be an edge case.

Screenshots:
slightdif

Troubleshooting information (do not edit):

Name: ‪pH Scale: Basics‬
URL: https://phet-dev.colorado.edu/html/ph-scale-basics/1.3.0-rc.1/phet/ph-scale-basics_all_phet.html
Version: 1.3.0-rc.1 2019-05-24 13:37:56 UTC
Features missing: touch
Flags: pixelRatioScaling
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/74.0.3729.169 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: 4096
Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 32)
Max viewport: 16384x16384
OES_texture_float: true
Dependencies JSON: {}

@pixelzoom
Copy link
Contributor

I add a no-op listener to the listbox to prevent event from going through it. @KatieWoe please verify in master. Assign back to me so that I can create branch "ph-scale-1.3" for sun and patch it there.

@KatieWoe
Copy link
Contributor Author

Still seems to happen on master

@KatieWoe KatieWoe assigned pixelzoom and unassigned KatieWoe May 30, 2019
pixelzoom added a commit to phetsims/sun that referenced this issue May 30, 2019
@pixelzoom
Copy link
Contributor

Hmnmm, sorry about that @KatieWoe, I must've been hallucinating. Previous commit reverted.

@pixelzoom pixelzoom changed the title Clicking on dropdown may activate dropper Clicking on listbox may activate dropper May 31, 2019
@pixelzoom
Copy link
Contributor

This is a problem in the RC (and master) but not in the published version. Similar to #86, this may be due to the new PressListener being used for the dropper's toggle button. Or it may be due to the ComboBox rewrite that occurred recently. As with #86, I'll need @jonathanolson's assistance.

@pixelzoom
Copy link
Contributor

The input listener that dismisses the listbox is in ComboBox, line 184:

      // @private Clicking anywhere other than the button or list box will hide the list box.
      this.clickToDismissListener = {
        down: event => {

          // Ignore if we click over the button, since the button will handle hiding the list.
          if ( !( event.trail.containsNode( this.button ) || event.trail.containsNode( this.listBox ) ) ) {
            this.hideListBox();
          }
        }
      };

@jonathanolson
Copy link
Contributor

If the combo box background should block clicks, presumably it should have pickable:true?

@pixelzoom
Copy link
Contributor

pixelzoom commented Jun 3, 2019

@jonathanolson do you mean the listbox background should have pickable:true? ComboBoxListBox extends Panel. I've tried passing pickable: true to the supertype, and I've tried adding a no-op listener like this.addInputListener( new PressListener() ).

@jonathanolson
Copy link
Contributor

Ahh interesting, it looks like Panel has an option backgroundPickable for the background.

Adding backgroundPickable: true to ComboBoxListBox seems to fix the issue. Is that something we can add in by default? (It seems good to me)

@jonathanolson jonathanolson removed their assignment Jun 3, 2019
@pixelzoom
Copy link
Contributor

Seems like ComboBoxListBox should have backgroundPickable: true set by default, so I've added it. And this fixes the issue.

@KatieWoe please verify, then assign back to me.

Note to self: I'll need to pick up this change in a "ph-scale-1.3" branch of sun.

@KatieWoe
Copy link
Contributor Author

Looks good on master

@KatieWoe KatieWoe assigned pixelzoom and unassigned KatieWoe Jun 17, 2019
@pixelzoom
Copy link
Contributor

pixelzoom commented Jun 17, 2019

Next step is to pick up phetsims/sun@35f7845 in a "ph-scale-1.3" branch of sun.

pixelzoom added a commit that referenced this issue Jun 17, 2019
pixelzoom added a commit to phetsims/ph-scale-basics that referenced this issue Jun 17, 2019
@pixelzoom
Copy link
Contributor

pixelzoom commented Jun 17, 2019

Patched in "ph-scale-1.3" branch of sun, ready to test in upcoming 1.3.0-rc.2.

@pixelzoom
Copy link
Contributor

Talked with @jonathanolson. I should also create and patch "ph-scale-basics-1.3" branch for sun.

@pixelzoom pixelzoom self-assigned this Jul 22, 2019
pixelzoom added a commit to phetsims/ph-scale-basics that referenced this issue Jul 23, 2019
@pixelzoom
Copy link
Contributor

pixelzoom commented Jul 23, 2019

Manually patched in "ph-scale-basics-1.3" branch of sun.

@pixelzoom
Copy link
Contributor

Verified by QA in phetsims/qa#374 and phetsims/qa#374. 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

3 participants