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

feat: Allow click handler to be specified in clickable component's options #6140

Merged
merged 2 commits into from
Aug 29, 2019

Conversation

mister-ben
Copy link
Contributor

Description

Allow a clickable component's click handler to be passed as a constructor option as a simple alternative to creating a custom component.
Makes for easier answers to "how do I create a button questions".

Specific Changes proposed

Add clickHandler as an option for ClickableComponent
All player controls override the handleClick method so are unaffected.

Requirements Checklist

  • Feature implemented / Bug fixed
  • If necessary, more likely in a feature request than a bug fix
    • Change has been verified in an actual browser (Chome, Firefox, IE)
    • Tests updated or fixed
    • Docs/guides updated
    • Example created (starter template on JSBin)
  • Reviewed by Two Core Contributors

@gkatsev gkatsev added the minor This PR can be added to a minor release. It should not be added to a patch release. label Jul 29, 2019
Copy link
Member

@gkatsev gkatsev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like a nice small improvement. Having the tests also verify that space/enter work, would be nice.

});
const el = testClickableComponent.el();

Events.trigger(el, 'click');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add a test that if we trigger a space or enter key on it, the click handler gets called?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor This PR can be added to a minor release. It should not be added to a patch release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants