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

Discuss onAccessibleClick and alternatives like exposing PressListener.a11yClickingProperty #872

Closed
zepumph opened this issue Sep 28, 2018 · 7 comments

Comments

@zepumph
Copy link
Member

zepumph commented Sep 28, 2018

This lengthy title stems from #831, and whether or not onAccessilbeClick is the best api for what we are trying to accomplish. Having a fully separate function that is only fired when a press occurs with a11y is nice, but I could also foresee wanting to have branching logic inside the same function that holds other input related code.

For example, something like:

    var pressListener = pushButtonModel.createListener( { 
      press: function() {
        if ( pressListener.a11yClickingProperty ) {
          // do a11y specific stuff
        }
        this.fire(); // always fire for both a11y and mouse
      },

This is just a bad example with pseudo code, but I think it conveys my point.

@jonathanolson
Copy link
Contributor

I was originally imagining that there would be an exposed flag/property like that.

@zepumph
Copy link
Member Author

zepumph commented Oct 2, 2018

@jessegreenberg do you have any concerns about this? I like the idea of simplifying the number of callbacks to pass into the PressListener constructor. We may find that we will still need options like this for the sun/ButtonModel hierarchy because we will only have access to the PressListener from ButtonModel.createListener. This seems like the most robust and straightforward way to proceed.

@jessegreenberg
Copy link
Contributor

No concerns at all, I definitely think this should be explored as a general improvement.

@zepumph
Copy link
Member Author

zepumph commented Oct 2, 2018

I'll take a look at this as part of my review of #831.

@zepumph
Copy link
Member Author

zepumph commented Oct 5, 2018

Worked on this with @jessegreenberg today. We removed accessibleClick listener and moved an a11yClicking check into the same listener as is fired with the mouse.

@jonathanolson would you please review this as part of you work with PressListener?

@zepumph
Copy link
Member Author

zepumph commented Oct 14, 2022

This issue is incredibly old. @jonathanolson and I have worked multiple times on PressListener since then. I'm going to close. Please reopen if you feel differently.

@zepumph
Copy link
Member Author

zepumph commented Feb 21, 2023

Oops, didn't close, but did just do another review of this code! Closing

@zepumph zepumph closed this as completed Feb 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants