fix(overlay): overlay class audits #6372 #8056
Conversation
amcdnl
requested review from
crisbeto,
josephperrott,
kara and
mmalerba
as code owners
googlebot
added
the
cla: yes
| @@ -35,7 +35,13 @@ export class MatSnackBarConfig { | |||
| duration?: number = 0; | |||
|
|
|||
| /** Extra CSS classes to be added to the snack bar container. */ | |||
| extraClasses?: string[]; | |||
| panelClass?: string[]; | |||
There was a problem hiding this comment.
I have two concerns with this,
-
This adds the class to the snackbar container, not the overlay panel. So it's slightly misleading and inconsistent with dialogs in that regard.
-
More importantly, I think
panelClassimplies that a single string would be sufficient. Sincestring[]is req'd, the snackbar container will convertpanelClass: "my-class"to<snack-bar-container class="m y - c l a s">...</snack-bar-container>I think the
MatSnackBar#_createOverlaycould use the overlay-ref logic to automatically handle this .
I know this PR is trying to only change the names, but I think this name change is misrepresenting unless it's also met with a change in functionality.
There was a problem hiding this comment.
Agreed; I didn't realize snackbar was string[] only right now. @amcdnl let's just change this one to string | string[] as well
src/lib/datepicker/datepicker.ts
Outdated
| @@ -167,6 +167,9 @@ export class MatDatepicker<D> implements OnDestroy { | |||
| */ | |||
| @Output() selectedChanged = new EventEmitter<D>(); | |||
|
|
|||
| /** Classes to be passed to the date picker panel. Supports the same syntax as `ngClass`. */ | |||
| @Input() panelClass: string|string[]|Set<string>|{[key: string]: any}; | |||
There was a problem hiding this comment.
I think Set<string>|{[key: string]: any} is a bit overkill. I'd rather stick with string | string[] for the time being.
src/lib/menu/menu-directive.ts
Outdated
| * @deprecated Use `panelClass` instead. | ||
| * @param classes list of class names | ||
| */ | ||
| @Input() classList = this.panelClass; |
There was a problem hiding this comment.
This needs a getter/setter that delegates to panelClass; this will only set the initial value
| @@ -35,7 +35,13 @@ export class MatSnackBarConfig { | |||
| duration?: number = 0; | |||
|
|
|||
| /** Extra CSS classes to be added to the snack bar container. */ | |||
| extraClasses?: string[]; | |||
| panelClass?: string[]; | |||
There was a problem hiding this comment.
Agreed; I didn't realize snackbar was string[] only right now. @amcdnl let's just change this one to string | string[] as well
| * Extra CSS classes to be added to the snack bar container. | ||
| * @deprecated Use `panelClass` instead. | ||
| */ | ||
| extraClasses?: string[] = this.panelClass; |
There was a problem hiding this comment.
This leaves the option here, but it will still break because it doesn't do anything now; the snack-bar needs to combine both until we remove this one.
There was a problem hiding this comment.
Done. Checkout what I did there, open to suggestions for better ideas.
src/lib/menu/menu-directive.ts
Outdated
| * menu template that displays in the overlay container. Otherwise, it's difficult | ||
| * to style the containing menu from outside the component. | ||
| * @deprecated Use `panelClass` instead. | ||
| * @param classes list of class names |
jelbourn
added
P1
|
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. |
Clean cut of #7454 that only changes names