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

Push paper-dialog forward. Still WIP. #313

Conversation

DanChadwick
Copy link
Contributor

  • Fixes escape to close.
  • Fixes click outside dialog to close. Required reinstating paper-dialog-container.
  • Convert named-by-string actions to action closures.
  • Remove vestigial paper-dialog-parent.
  • Improves documentation and dummy app.
  • Update other components to conform to prior API change to paper-backdrop tap action.
  • Temporarily disabled transitions. Transitions do not work and prevent functioning. When transitions work again. the commit for this should be reversed. Changes are marked with TODO.
  • Added "prompt dialog" example.
  • Applies fixed positioning to backdrop if no parent is defined. This satisfies the automated test and AM behavior, but at least on desktop, I can detect no different in actual browser behavior.
  • Fixed fullscreen (test passes now).
  • Updated dummy app to show primary-colored buttons and demonstrate fullscreen.

@DanChadwick
Copy link
Contributor Author

Updated top comment to reflect the last 4 commits. This branch should now be functional again, but with transitions disabled.

classNames: ['md-default-theme'],
classNameBindings: ['contentOverflow:md-content-overflow', 'fullscreen:md-dialog-fullscreen'],
classNames: ['md-default-theme' , 'md-transition-in' /* TODO */ ],
classNameBindings: ['contentOverflow:md-content-overflow', 'dialogComponent.fullscreen:md-dialog-fullscreen'],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can't we get rid of dialogComponent properties by invoking paper-dialog-inner as

{{#paper-dialog-inner fullscreen=fullscreen clickOutsideToClose=clickOutsideToClose ...

?

Since paper-dialog-inner is a private component, we can use it like we want to. No need for a nearestOfType dependency, probably.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We use dialogComponent in paper-inner-dialog for 4 different things. We could pass it by binding this, we could bind all four things we use, or we can keep what we have. I thought the discussion from tabs was that it was better to connect the two components than to bind a bunch of properties between two components.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That discussion does not apply for private components.
We had to resort to such mechanisms because we can't expect the user to pass them by us.

… than use nearestOfType to find the dialog component.
miguelcobain added a commit that referenced this pull request Mar 21, 2016
Push paper-dialog forward. Still WIP.
@miguelcobain miguelcobain merged commit b0b39ac into adopted-ember-addons:dialogs-jj-abrams Mar 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants