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

dialog: autoWrap in $mdDialog does not work #10508

Closed
yueluog opened this issue Mar 17, 2017 · 3 comments
Closed

dialog: autoWrap in $mdDialog does not work #10508

yueluog opened this issue Mar 17, 2017 · 3 comments
Assignees
Labels
g3: reported The issue was reported by an internal or external product team. P2: required Issues that must be fixed. resolution: works as expected The functionality works as designed and documented.

Comments

@yueluog
Copy link

yueluog commented Mar 17, 2017

Actual Behavior:

  • What is the issue? *
    According to $mdDialog spec, If we have a customized directive that expands to md-dialog, we can set autoWrap to false. However, this does not always work. The dialog would not show up.

(If we set autoWrap to true, it works. But two levels of md-dialog are generated, and the caller has no way to style the first level of md-dialog. This makes attribute flex="xxx" useless.)

  • What is the expected behavior?
    Dialog should open.

Demo:
http://plnkr.co/edit/8cMGwKAstTXchz6juEXY?p=preview
No dialog shows up when the button is clicked. And there is error on console.

Uncaught TypeError: node.hasAttribute is not a function
    at expect (angular-material.js:2016)
    at angular-material.js:2035

This demo is based on another bug report: #8409 (comment). The only change is that I added "autoWrap: false".
It is possible that there is some link between that bug and this one.

AngularJS Versions: *

  • AngularJS Version: 1.5.9
  • AngularJS Material Version: 1.1

Additional Information:

  • Browser Type: Chrome
  • Browser Version: * 56.0.2924.87
  • OS: * Ubuntu
  • Stack Traces:
@jelbourn jelbourn added the g3: reported The issue was reported by an internal or external product team. label Mar 17, 2017
@yueluog
Copy link
Author

yueluog commented Mar 20, 2017

I found some information that may be related: http://stackoverflow.com/questions/32097593/angularjs-templateurl-vs-template-scope-issue. The answer in the Stackoverflow is not directly to this bug, but I think the principal applies. My hypothesis is that when templateUrl is used, this bug happens because during the compilation, the custom directive is not expanded yet to md-dialog. Whereas if template is used, then the compilation will expand the custom directive to md-dialog early and ngmaterial code will see that md-dialog is already there, and honor autoWrap.

@Splaktar Splaktar self-assigned this Jan 16, 2019
@Splaktar Splaktar added this to the 1.1.13 milestone Jan 16, 2019
@Splaktar Splaktar modified the milestones: 1.1.13, 1.1.14 Feb 10, 2019
@Splaktar Splaktar changed the title autoWrap in $mdDialog does not work dialog: autoWrap in $mdDialog does not work Feb 11, 2019
@Splaktar
Copy link
Member

This issue is due to an exception in the MdAriaService's expect function that checks if the expected aria-label attribute exists on the target element or child:

/**
* Check if expected attribute has been specified on the target element or child
* @param element
* @param attrName
* @param {optional} defaultValue What to set the attr to if no value is found
*/
function expect(element, attrName, defaultValue) {
var node = angular.element(element)[0] || element;
// if node exists and neither it nor its children have the attribute
if (node &&
((!node.hasAttribute(attrName) ||
node.getAttribute(attrName).length === 0) &&
!childHasAttribute(node, attrName))) {

In this demo, it gets passed two elements. It checks the md-subheader which has no aria-label and it checks an empty JQLite object which has no hasAttribute function and throws the exception.

If autoWrap is true, then the elements passed in are an md-dialog and the md-subheader.

So it looks like the Aria checks don't pass in the proper parent element when autoWrap is false.

@Splaktar Splaktar added type: bug P2: required Issues that must be fixed. for: internal contributor The team will address this issue and community PRs are not requested. for: external contributor needs: investigation The cause of this issue is not well understood and needs to be investigated by the team or community labels Feb 11, 2019
@Splaktar Splaktar modified the milestones: 1.1.14, g3: sync Feb 15, 2019
@Splaktar Splaktar modified the milestones: g3: sync, 1.1.23 Apr 30, 2020
@Splaktar Splaktar modified the milestones: 1.1.23, 1.2.0 Jun 8, 2020
@Splaktar Splaktar modified the milestones: 1.2.0, 1.2.1 Jul 29, 2020
@Splaktar Splaktar modified the milestones: 1.2.1, 1.2.2 Sep 14, 2020
@Splaktar
Copy link
Member

I've migrated this demo to StackBlitz and updated it to not rely on .component() as that is unsupported (#8409). This new demo demonstrates that autoWrap: false is working properly when using a supported .directive().

I'm going to close this, but I'll mention in #8409 that we need to ensure that autoWrap works properly.

@Splaktar Splaktar removed for: external contributor for: internal contributor The team will address this issue and community PRs are not requested. needs: investigation The cause of this issue is not well understood and needs to be investigated by the team or community type: bug labels Dec 15, 2020
@Splaktar Splaktar added the resolution: works as expected The functionality works as designed and documented. label Dec 15, 2020
@Splaktar Splaktar removed this from the 1.2.2 milestone Dec 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
g3: reported The issue was reported by an internal or external product team. P2: required Issues that must be fixed. resolution: works as expected The functionality works as designed and documented.
Projects
None yet
Development

No branches or pull requests

3 participants