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

fix(cdk/dialog): add container structural styles #24905

Merged
merged 1 commit into from
May 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/cdk/dialog/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ load(
"ng_module",
"ng_test_library",
"ng_web_test_suite",
"sass_binary",
)

package(default_visibility = ["//visibility:public"])
Expand All @@ -14,7 +15,7 @@ ng_module(
["**/*.ts"],
exclude = ["**/*.spec.ts"],
),
assets = glob(["**/*.html"]),
assets = [":dialog-container.css"] + glob(["**/*.html"]),
deps = [
"//src:dev_mode_types",
"//src/cdk/a11y",
Expand Down Expand Up @@ -61,3 +62,8 @@ filegroup(
name = "source-files",
srcs = glob(["**/*.ts"]),
)

sass_binary(
name = "dialog_container_scss",
src = "dialog-container.scss",
)
13 changes: 13 additions & 0 deletions src/cdk/dialog/dialog-container.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
.cdk-dialog-container {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might add a comment for the whole class that mentions the intent is to provide the bare minimum styles to make the container behave reasonably, with most of the customization expected to be done by the consumer of CdkDialog

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The consumer isn't expected to customize this component, but the one inside it or the container around it.

// The container is a custom node so it'll be `display: inline` by default.
display: block;

// The dialog container should completely fill its parent overlay element.
width: 100%;
height: 100%;

// Since the dialog won't stretch to fit the parent, if the height
// isn't set, we have to inherit the min and max values explicitly.
min-height: inherit;
max-height: inherit;
}
1 change: 1 addition & 0 deletions src/cdk/dialog/dialog-container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export function throwDialogContentAlreadyAttachedError() {
@Component({
selector: 'cdk-dialog-container',
templateUrl: './dialog-container.html',
styleUrls: ['dialog-container.css'],
encapsulation: ViewEncapsulation.None,
// Using OnPush for dialogs caused some G3 sync issues. Disabled until we can track them down.
// tslint:disable-next-line:validate-decorators
Expand Down
6 changes: 0 additions & 6 deletions src/dev-app/cdk-dialog/dialog-demo.scss
Original file line number Diff line number Diff line change
@@ -1,9 +1,3 @@
.demo-cdk-dialog {
background: white;
padding: 20px;
border-radius: 8px;
}

.demo-container {
button, label {
margin-right: 8px;
Expand Down
20 changes: 18 additions & 2 deletions src/dev-app/cdk-dialog/dialog-demo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,24 @@ export class DialogDemo {
<button (click)="temporarilyHide()">Hide for 2 seconds</button>
</div>
`,
encapsulation: ViewEncapsulation.None,
styles: [`.hidden-dialog { opacity: 0; }`],
styles: [
`
:host {
background: white;
padding: 20px;
border-radius: 8px;
display: block;
width: 100%;
height: 100%;
min-width: inherit;
min-height: inherit;
}
:host-context(.hidden-dialog) {
opacity: 0;
}
`,
],
standalone: true,
})
export class JazzDialog {
Expand Down