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

Dialogs: improve API #171

Closed
quanterion opened this issue Nov 24, 2016 · 2 comments
Closed

Dialogs: improve API #171

quanterion opened this issue Nov 24, 2016 · 2 comments
Assignees
Milestone

Comments

@quanterion
Copy link
Contributor

You makes it really simple to open simple dialogs. I suggest a few additions:

  1. You have created a service with alert/prompt/confirm methods. If I need to open custom-component dialog I would need to inject MdDialog service to open it. So it often ends up with injecting two similar services. It would be great to have simple open method from MdDialog in TdDialogService to avoid double injection.

  2. Add simple alternatives to alert/prompt/confirm that way:
    prompt(string): Observable<string>;
    It would allow even simpler way to use dialogs!

PS: I can make a PR for this if you like the ideas!

@emoralesb05
Copy link
Contributor

emoralesb05 commented Nov 25, 2016

I agree, it would be simple to create wrapper methods to the MdDialog methods inside TdDialogService so we dont have to inject both. e.g. tdDialogService.open(CustomDialog, config) or something like that

@emoralesb05 emoralesb05 added this to the Alpha 0.10 milestone Nov 25, 2016
@emoralesb05 emoralesb05 self-assigned this Dec 13, 2016
kyleledbetter pushed a commit that referenced this issue Dec 16, 2016
…Dialog. closes(#170 and #171) (#190)

* added a11y to prompt dialog for enter keypress and left/right arrows  when buttons are focused

* added a11y to confirm dialog for left/right arrows  when buttons are focused

* prevent enter event propagation outside of dialog

* proxy the open and closeAll methods in the tdDialogService

* added open and closeAll usage in the docs
@emoralesb05
Copy link
Contributor

Added point no 1 and will be available for 0.10.

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

2 participants