Skip to content

fix(material-experimental/mdc-list): avoid style conflicts with other list-based components #22562

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

fix(material-experimental/mdc-list): avoid style conflicts with other list-based components #22562

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 5 additions & 3 deletions src/material-experimental/mdc-list/_list-theme.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,11 @@
@mixin color($config-or-theme) {
$config: theming.get-color-config($config-or-theme);

// MDC's state styles are tied in with their ripple. Since we don't use the MDC
// ripple, we need to add the hover, focus and selected states manually.
@include interactive-list-theme.private-interactive-list-item-state-colors($config);
.mat-mdc-list-base {
// MDC's state styles are tied in with their ripple. Since we don't use the MDC
// ripple, we need to add the hover, focus and selected states manually.
@include interactive-list-theme.private-interactive-list-item-state-colors($config);
}

@include mdc-helpers.mat-using-mdc-theme($config) {
@include mdc-list.deprecated-without-ripple($query: mdc-helpers.$mat-theme-styles-query);
Expand Down
4 changes: 2 additions & 2 deletions src/material-experimental/mdc-list/action-list.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ import {MatListBase} from './list-base';
@Component({
selector: 'mat-action-list',
exportAs: 'matActionList',
template: '<ng-content></ng-content>',
template: '<div class="mdc-deprecated-list"><ng-content></ng-content></div>',
host: {
'class': 'mat-mdc-action-list mat-mdc-list-base mdc-deprecated-list',
'class': 'mat-mdc-action-list mat-mdc-list-base',
},
styleUrls: ['list.css'],
encapsulation: ViewEncapsulation.None,
Expand Down
52 changes: 28 additions & 24 deletions src/material-experimental/mdc-list/list.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,32 @@
@use '../../material/core/style/layout-common';
@use '../../cdk/a11y';

@include mdc-list.deprecated-without-ripple($query: mdc-helpers.$mat-base-styles-query);

// MDC expects the list element to be a `<ul>`, since we use `<mat-list>` instead we need to
// explicitly set `display: block`
.mat-mdc-list-base {
// This mixin needs to be nested so that styles don't leak into other
// components based on the MDC list (e.g. select, autocomplete).
@include mdc-list.deprecated-without-ripple($query: mdc-helpers.$mat-base-styles-query);
display: block;

// MDC does have 2-line support, but it's a per-list setting, whereas ours applies to individual
// items. Therefore, we need to add our own styles.
.mat-mdc-2-line {
height: 72px;

.mdc-deprecated-list-item__text {
align-self: flex-start;
}
}

// MDC does not support more than 2 lines, so we need to add our own styles.
.mat-mdc-3-line {
height: 88px;

.mdc-deprecated-list-item__text {
align-self: flex-start;
}
}
}

// .mdc-deprecated-list-item__primary-text adds its own margin settings, so only reset if it doesn't
Expand All @@ -24,25 +44,6 @@
}


// MDC does have 2-line support, but it's a per-list setting, whereas ours applies to individual
// items. Therefore, we need to add our own styles.
.mat-mdc-2-line {
height: 72px;

.mdc-deprecated-list-item__text {
align-self: flex-start;
}
}

// MDC does not support more than 2 lines, so we need to add our own styles.
.mat-mdc-3-line {
height: 88px;

.mdc-deprecated-list-item__text {
align-self: flex-start;
}
}

// MDC supports avatars, but it's a per-list setting, whereas ours applies to individual
// items. Therefore, we need to add our own styles.
.mat-mdc-list-avatar {
Expand All @@ -51,9 +52,12 @@
$size: 40px;
$margin-value: 72px - mdc-list.$deprecated-side-padding - $size;

width: $size;
height: $size;
border-radius: 50%;
// These styles need the extra specificity in order to override the ones from MDC.
.mat-mdc-list-base & {
width: $size;
height: $size;
border-radius: 50%;
}

// Avatars that come before the text have the .mdc-deprecated-list-item__graphic class.
// MDC's .mdc-deprecated-list--avatar-list class would normally apply overrides to set the
Expand Down
4 changes: 2 additions & 2 deletions src/material-experimental/mdc-list/list.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ import {MatListBase, MatListItemBase} from './list-base';
@Component({
selector: 'mat-list',
exportAs: 'matList',
template: '<ng-content></ng-content>',
template: '<div class="mdc-deprecated-list"><ng-content></ng-content></div>',
host: {
'class': 'mat-mdc-list mat-mdc-list-base mdc-deprecated-list',
'class': 'mat-mdc-list mat-mdc-list-base',
Copy link
Member

Choose a reason for hiding this comment

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

Is there a better way we can avoid style conflicts? I think this makes it a bit inconvenient in terms of instantiating the MDCList foundation, and declaring the adapter. I'm not sure if this currently has any a11y impact.

components/src/material-experimental/mdc-list/interactive-list-base.ts

Line 160 in ddbfe11

export function getInteractiveListAdapter(

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed that this isn't great, but the alternative would be to fix it in the select, autocomplete and menu specifically, but then it could still conflict with other list-based components in the future or other non-Material components the user might have on the page.

Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably change the adapter to apply any classes, attributes, etc to the div. The only other way I can think to do this is if MDC changed their mixins to allow us to add to the root selector:

@mixin deprecated-without-ripple(..., $root: '') {
  #{$root}.mdc-deprecated-list {
    ...
  }
}

Copy link
Member

@devversion devversion Apr 26, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

@mmalerba Yeah, I thought that too, but then realized that this could break a11y because the MDCList "root" usually receives the aria- attributes from the foundation. Personally I'd prefer what you showed. We might be able to convince MDC to support this because list is commonly used in various other components too, and it makes style isolation easier.

},
styleUrls: ['list.css'],
encapsulation: ViewEncapsulation.None,
Expand Down
4 changes: 2 additions & 2 deletions src/material-experimental/mdc-list/nav-list.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ import {MatListBase} from './list-base';
@Component({
selector: 'mat-nav-list',
exportAs: 'matNavList',
template: '<ng-content></ng-content>',
template: '<div class="mdc-deprecated-list"><ng-content></ng-content></div>',
host: {
'class': 'mat-mdc-nav-list mat-mdc-list-base mdc-deprecated-list',
'class': 'mat-mdc-nav-list mat-mdc-list-base',
},
styleUrls: ['list.css'],
encapsulation: ViewEncapsulation.None,
Expand Down
4 changes: 2 additions & 2 deletions src/material-experimental/mdc-list/selection-list.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,11 @@ export class MatSelectionListChange {
selector: 'mat-selection-list',
exportAs: 'matSelectionList',
host: {
'class': 'mat-mdc-selection-list mat-mdc-list-base mdc-deprecated-list',
'class': 'mat-mdc-selection-list mat-mdc-list-base',
'role': 'listbox',
'[attr.aria-multiselectable]': 'multiple',
},
template: '<ng-content></ng-content>',
template: '<div class="mdc-deprecated-list"><ng-content></ng-content></div>',
styleUrls: ['list.css'],
encapsulation: ViewEncapsulation.None,
providers: [
Expand Down