feat(dialog): add hasBackdrop and backdropClass options to dialog config #2822
Conversation
googlebot
added
the
cla: yes
|
It looks like saucelabs timed out in the last build, not sure if someone here has the ability to redo the build. |
|
Reset the tests for you. |
|
Are there any other steps I should take for this pull request? Or is the only thing left to do wait for someone from the material2 team to take a look? |
|
It still needs to be reviewed. Also I can see how the |
|
I was actually thinking hasBackdrop is probably less useful. When hasBackdrop is false you also lose the ability to close when clicking away, which undermines disableClose. However, with backdropClass you can make the backdrop transparent and then control whether you want the ability to close or not with disableClose. backdropClass only replaces the default The library is already offering the ability to set the position of a dialog, allowing people to use it for more than just a simple standard dialog, so I think allowing custom styling of the backdrop is definitely useful. |
|
@crisbeto Is it possible to get this reviewed or at least discussed? Beta.3 breaks the hack I was using to have some dialogs with transparent backgrounds and it looks like at least one other person that found this is also interested (looking at the reaction to my last comment). From what I have researched the Material Guidelines don't specify/require a specific background overlay color. The API already gives the ability to customize position, width, height, so obviously it's intended to be used in many ways. The Overlay API already supports the ability to apply custom classes or just use the existing transparent class. Why should we be forced to use the translucent black overlay? This is an incredibly easy change and I can fix this PR to get it up to date with the latest master, but it would be appreciated if I could get some input from the maintainers, even if it's just a "Yes" or "No." |
|
Here is an example of my use case. The dialog below is our "Share" dialog. It opens at the position of the share icon when you click, using the Material API's ability to set the position of the dialog. Since this dialog acts similar to a menu, we want a transparent overlay backdrop. Currently this is not possible with the Material2 API. The following dialog is our Login Dialog. This is more like a traditional modal/dialog and should have a black translucent overlay backdrop. This is currently the only backdrop allowed using the Material2 API. |
src/lib/dialog/dialog.ts
Outdated
| @@ -161,7 +161,10 @@ export class MdDialog { | |||
| let strategy = this._overlay.position().global(); | |||
| let position = dialogConfig.position; | |||
|
|
|||
| state.hasBackdrop = true; | |||
| state.hasBackdrop = dialogConfig.hasBackdrop; | |||
| if (dialogConfig.backdropClass && dialogConfig.backdropClass !== '') { | |||
There was a problem hiding this comment.
The extra !== '' shouldn't be necessary
Implement hasBackdrop as laid out in the MdDialog design document. Add backdropClass option to override default `cdk-overlay-dark-backdrop` class, allowing custom styling to be applied. Closes angular#2806
YeomansIII
force-pushed
the
dialog-backdrop-config-feature
branch
from
5038744
to
e6a9323
Compare
|
@jelbourn Resolved conflicts with master and made changes based on your suggestion. |
jelbourn
added
pr: lgtm
action: merge
|
@YeomansIII Thank you for your great job. Do we know briefly when this will be released? |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Implement hasBackdrop as laid out in the MdDialog design document. Add backdropClass option to override default
cdk-overlay-dark-backdropclass, allowing custom styling to be applied.Closes #2806
Somewhat related #1584