Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

$mdDialog doesn't play well with forms #1190

Closed
wmertens opened this issue Jan 14, 2015 · 17 comments
Closed

$mdDialog doesn't play well with forms #1190

wmertens opened this issue Jan 14, 2015 · 17 comments

Comments

@wmertens
Copy link

See http://jsfiddle.net/p02Ls1dh/3/

mdDialog would be perfect for a quick entry panel, but it sets focus on the last button by default and if you try to use <form> for handling submit it breaks.

So maybe $mdDialog should:

  • support a .md-dialog-focus class to set the focus
  • bind the enter key for the .dialog-close button

PS: I needed to put type=button on the cancel md-button so the form wouldn't click Cancel but Submit when typing <Enter>. Is that a bug with md-button or is that expected?

@wmertens wmertens changed the title mdDialog doesn't play well with forms $mdDialog doesn't play well with forms Jan 14, 2015
@marcysutton
Copy link
Contributor

Focus is set in the modal for accessibility purposes, but it does make sense to make the focusable element configurable. Maybe people want to send focus to a heading with tabIndex="-1" or some other element.

submit is the default type of button, so having to include type="button" to not submit a form is expected. https://developer.mozilla.org/ru/docs/Web/HTML/Element/button (Also, space bar is the expected key when working with buttons.)

@wmertens
Copy link
Author

Alright, supporting tabIndex="-1" sounds good to me, or maybe even the element with the lowest tabIndex.

As for the key, I meant using the Enter key while the focus is elsewhere in the dialog. It would obviate the need of a <form> tag. The Esc key is already captured, so Enter would be nice too. Right now it only works because the close button is getting focus on show.

@wmertens
Copy link
Author

I thought about this some more and I think it would be great if angular could facilitate the HTML5 http://caniuse.com/#feat=autofocus attribute.
This means that on element create, it sets focus on that element, and directives like $mdDialog should not steal focus if there's an element with autofocus set.

The directive is very simple: https://gist.github.com/mlynch/dd407b93ed288d499778 - should this be part of Angular proper? That leaves just the not-stealing-focus in $mdDialog.

@wmertens
Copy link
Author

I just created angular/angular#455 for the support in Angular proper.

@gustavohenke
Copy link

Beware that angular/angular is Angular 2.0 ;)

@wmertens
Copy link
Author

@gustavohenke argh thanks, I was wondering why there was no other issue mentioning it 😅. angular/angular.js#10833 is the new one.

@naomiblack naomiblack modified the milestones: 0.8.0, 0.7.1 Jan 26, 2015
@rschmukler
Copy link
Contributor

Deferring to this 0.9 while we wait to hear back from the angular team. @robertmesserle any word?

@rschmukler rschmukler modified the milestones: 0.9.0, 0.8.0-rc1 Feb 8, 2015
@mvidailhet
Copy link

In the meantime, I think it would be great to have an option to deactivate the autofocus :)

@mvidailhet
Copy link

After some thoughts, I think the easiest and more generic way to handle this is to broadcast an event 'scope.$broadcast('dialog-showed');' at the end of the onShow function of the dialog. After catching this event, we would be able to change the focus or do whatever we want on the dialog. What do you think ?

the line to write this broadcast: https://github.com/angular/material/blob/master/src/components/dialog/dialog.js#L441

I can do a PR with the modifications if you want.

@wmertens
Copy link
Author

I would still prefer declarative focus, either by the workaround of tab
order or by the autofocus attribute... Listening for events is so kludgy
:-(

On Mon, Feb 16, 2015, 7:20 PM Michel Vidailhet notifications@github.com
wrote:

After some thoughts, I think the easiest and more generic way to handle
this is to broadcast an event 'scope.$broadcast('dialog-showed');' at the
end of the onShow function of the dialog. After catching this event, we
would be able to change the focus or do whatever we want on the dialog.
What do you think ?

the line to write this broadcast:
https://github.com/angular/material/blob/master/src/components/dialog/dialog.js#L441

I can do a PR with the modifications if you want.


Reply to this email directly or view it on GitHub
#1190 (comment).

@rschmukler rschmukler added the needs: team discussion This issue requires a decision from the team before moving forward. label Apr 2, 2015
@robertmesserle robertmesserle modified the milestones: 0.10.0, 0.9.0 Apr 2, 2015
@sysupbda
Copy link

I am with wmertens on this. I would also prefer declarative.

Until we actually agree on the proper solution, could we maybe already all agree that it is not ok for the closeButton to steal the focus? At least each person could implement their solution until there is a "beautiful" way of setting this up. Right now it just steals away the focus and work arounds seem truly ugly. Mean-time I am just commenting out line 375 of dialog/dialog.js.. :)

Thanks for the beautiful work. I really am enjoying using md-angular.

@marcysutton
Copy link
Contributor

There is already an option in dialog setup called focusOnOpen, which you can set to false if you want to change the behavior. We recently added a md-sidenav-focus child directive to sidenavs, sounds like it would be a great addition to dialogs and bottom sheets as well.

@ThomasBurleson
Copy link
Contributor

Reference #2313.

@sysupbda
Copy link

Thank you marcysutton. Happy there is focusOnOpen and that we are looking at a md-sidenav-focus like solution. A small side note is that I could not find the focusOnOpen change in the CHANGELOG.md. Should it maybe go in there?

@marcysutton
Copy link
Contributor

Not a bad idea. It was documented in a commit, although explicitly calling out focusOnOpen could have made it more clear. It is also documented in the Dialog Service API doc. 0b0ed23

@ThomasBurleson ThomasBurleson modified the milestones: 0.10.0, 0.11.0 Jun 16, 2015
@rschmukler rschmukler removed the needs: team discussion This issue requires a decision from the team before moving forward. label Jul 9, 2015
@ThomasBurleson
Copy link
Contributor

We will be adding a md-autofocus directive to allow SideNav, DIalog, and Bottomsheet to specify a custom autofocus target.

@ThomasBurleson
Copy link
Contributor

Updated with SHA 5fc572d

kennethcachia pushed a commit to kennethcachia/material that referenced this issue Sep 23, 2015
…md-auto-focus`

Provide a consistent approach to optionally specify child element that should be focused for any component. When the owning component is ready, use `$mdUtil.findFocusTarget()`

* Bottomsheet and sideNav allows any child element to be the auto-focus target; using `md-auto-focus`
  * Sidenav Basic Usage demo specifies `md-auto-focus` on the input form element.
  * Bottomsheet Basic Usage demo shows expression use with `md-auto-focus` to conditional
focus an element within ng-repeat.
* Dialog allows custom dialog content to specify an `md-auto-focus` target for any content element.
  *  Basic Demo uses `md-auto-focus` on first action button

BREAKING CHANGE: md-autocomplete's `md-autofocus` has changed to `md-enable-autofocus`; to differentiate use with the new `md-auto-focus` indicator.

Change your code from this:

    ```html
    <md-autocomplete md-autofocus>
    ```

    To this:

    ```html
        <md-autocomplete md-enable-autofocus>
    ```

Fixes angular#1190.
@Splaktar Splaktar modified the milestones: 1.0-rc1, 0.11.0 Sep 21, 2018
@angular angular locked as resolved and limited conversation to collaborators Sep 21, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

10 participants