dialog: theme not inheriting properly when changing themes
Bug, feature request, or proposal:
Bug
What is the expected behavior?
The dialog will take on the theme that is set on its parent element (or somewhere in the ascendants).
What is the current behavior?
If changing the theme, the dialog still uses the previous theme.
CodePen and steps to reproduce the issue:
CodePen Demo which shows your issue: https://codepen.io/anon/pen/WzmmKx
Detailed Reproduction Steps:
- set a theme
- open dialog WITHOUT targetEvent set with an md-primary button and view theme
- switch theme using theme provider/service
- open dialog and see that theme did not update
What is the use-case or motivation for changing an existing behavior?
It essentially renders theme changing useless since the interim elements don't respect the change.
Which versions of AngularJS, Material, OS, and browsers are affected?
AngularJS Material >= 1.1.2 AngularJS >= 1.6.0
Is there anything else we should know? Stack Traces, Screenshots, etc.
Likely something in the following changes that broke it: https://github.com/angular/material/pull/9762/files#diff-58cbee594b2fabf2c78b6d283750e535R806
Originally I did not realize this, but it appears a work around is setting the targetEvent when opening a dialog via the $mdDialog service. Not sure why/how this is related, and it is not documented anywhere that I've found.
I updated the CodePen to 1.1.8 and I still see the issue. Thank you for reporting this and for creating the CodePen.
Originally I did not realize this, but it appears a work around is setting the targetEvent when opening a dialog via the $mdDialog service. Not sure why/how this is related, and it is not documented anywhere that I've found.
Another workaround – just set the theme explicitly, like so:
let alert = $mdDialog.alert({
title: 'Themed',
textContent: 'This dialog is correctly themed!',
ok: 'Close',
theme: $scope.theme
});
$mdDialog.show(alert);
$mdDialogPreset#theme is documented under the preset creators, e.g. $mdDialog.alert().
This workaround also works when you can't set targetEvent (this was how I found it out), and it's more stable as it doesn't rely on the not well understood connection with targetEvent, which might disappear in next versions of AngularJS Material.
$mdDialog's Theme Inheritance Demo uses the targetEvent API, which is required for theme inheritance.
However, while this API is documented, it isn't documented that this API is required for theme inheritance. A note about this needs to be added to the $mdDialog service docs. I don't think that it makes sense directly on the targetEvent API. We should probably add a dialog theming example to these docs and a note about specifying targetEvent being required.
We should also have a brief example of theming dialogs using the theme API w/o inheritance (as mentioned in the previous comment).
So do I understand correctly that theme inheritance works like theme is inherited from the element which corresponds to the DOMClickEvent passed as targetEvent?
Anyway, information about dialogs' theme inheritance mechanism would be nice in the theming docs as well, as for example for me it was surprising that I set an md-theme on my body and it didn't propagate to the dialogs, despite that they're children of body.
(The logic of my app is so that it doesn't make sense for my dialogs to have a targetEvent. If I understand correctly, in that case theme inheritance is not defined according to AngularJS Material's system, so I have to explicitly set the theme, which is fine, just it would be nice if this was documented.)
As I think about it, theme inheritance via targetEvent is actually reasonable, as there might be multiple themes present among the elements of the page, and then $mdDialog wouldn't know which theme the dialog should inherit. Although perhaps it could be set to fall back to parent's theme in case targetEvent is not set, and then the explicit setting I mentioned above wouldn't be necessary.
So do I understand correctly that theme inheritance works like theme is inherited from the element which corresponds to the DOMClickEvent passed as targetEvent?
Yes, see https://github.com/angular/material/blob/a13722ee69ab01babefe1f0bcc5af77f55a3f76c/src/components/dialog/dialog.js#L825-L839
Agreed that it should be mentioned with a note/callout in the theming guide.