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

Dialog API buff-up. #325

Merged

Conversation

DanChadwick
Copy link
Contributor

parent, origin, openFrom, closeTo are all optional and all take a jQuery selector or jQuery object.

I found the original API inconsistent and inconvenient in actual use. This PR normalized the API and cleans up some old code remnants.

Note that parent is not an id, but a jQuery selector or object. An id is created, if needed, for {{ember-wormhole}}.

@DanChadwick
Copy link
Contributor Author

Test failure seems to be a travis error. It can't install phantomjs for ember-release, but default, beta, and canary all pass fine.

@miguelcobain miguelcobain added this to the 1.0 milestone Mar 28, 2016

constants: service(),

didInsertElement() {
if (this.get('escapeToClose')) {
let parent = this.get('parentElementSelector');
$(parent).on(`keydown.${this.elementId}`, (e) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why this change?

+this.get('defaultedParent')
-this.get('parentElementSelector')

defaultedParent is the new parentElementSelector?

Copy link
Contributor

Choose a reason for hiding this comment

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

+1, parentElement || parentElementSelector is a very clear descriptor.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not complaining about the names. Just trying to understand.

@miguelcobain miguelcobain merged commit 5e72f33 into adopted-ember-addons:wip/v1.0.0 Mar 28, 2016
@DanChadwick DanChadwick deleted the dialog-buffup branch March 28, 2016 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants