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

Rename "action" handler to "onClickOutside" #47

Merged
merged 6 commits into from
Aug 5, 2019
Merged

Rename "action" handler to "onClickOutside" #47

merged 6 commits into from
Aug 5, 2019

Conversation

AndreJoaquim
Copy link
Contributor

@AndreJoaquim AndreJoaquim commented Mar 11, 2019

Closes #45.

I've decided to supersede onClickOutside. This means that if onClickOutside is passed, action is ignored. If both are passed, only onClickOutside is called. If only action is passed, only action is called.

I've added two deprecation warnings:

  1. When both onClickOutside and action handlers are passed and are functions, it warns that only onClickOutside should be defined
  2. When only action handler is passed, it warns that it's deprecated and onClickOutside should be used instead

I've added and adapted the test suite to include the new behaviours, as well as adapting the README.md

@zeppelin
Copy link
Owner

Great work, almost clicked the merge button! :) But then it hit me if you could camelcase except-selector (exceptSelector) as well? It'd be nice to remain consistent with the casing of the onClickOutside argument.

@AndreJoaquim
Copy link
Contributor Author

@zeppelin Totally agree! Will do that today :)

@AndreJoaquim
Copy link
Contributor Author

AndreJoaquim commented Mar 19, 2019

@zeppelin Sorry, I've not yet managed to pick it up, but it's not forgotten!

@zeppelin
Copy link
Owner

@AndreJoaquim no worries, we're not in a hurry ;)

@ondrejsevcik
Copy link

hey guys, did you consider using deprecatingAlias for this? https://api.emberjs.com/ember/3.9/functions/@ember%2Fobject%2Fcomputed/deprecatingAlias

I think it may serve its purpose in this case.

- Deprecate `except-selector` in favor of `exceptSelector`
- Update how `action` deprecation is handled
@zeppelin
Copy link
Owner

zeppelin commented Aug 5, 2019

@AndreJoaquim I've deprecated except-selector in favor of exceptSelector, also removed the action<onClickOutside precedence, because deprecatingAlias (thanks, @ondrejsevcik!) does not allow to have that. In exchange, we have a deprecation workflow more aligned with the rest of the ecosystem.

Thank you for the great contribution!

Next up: element modifiers :)

@zeppelin zeppelin merged commit 6c5738a into zeppelin:master Aug 5, 2019
@AndreJoaquim AndreJoaquim deleted the rename-action-to-on-click-outside branch August 5, 2019 13:23
@AndreJoaquim
Copy link
Contributor Author

@zeppelin Cool!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Renaming component's callback on clickOutside
3 participants