Skip to content

Conversation

devversion
Copy link
Member

Similarly to mdc-density, we add a mixin for all of the density styles
that will exist in src/material.

Additionally, mdc-typography and mdc-theming mixins are simplified
now that the all-theme mixins can control individual theming system
parts. This does not work for @angular/material typography due to
a cyclic dependency which we could eliminate if we require people
to explicitly import the file where the mat-core mixin originates from.

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Mar 9, 2020
// discussion as it would introduce a circular dependency for typography because:
// -> `all-theme` -> `mat-core` -> `all-typography` -> `all-theme`.
// This happens because developers rely on the `mat-core` to be available when
// the `all-theme` file is imported.
Copy link
Member Author

Choose a reason for hiding this comment

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

A possible solution would be that developers need to explicitly import the file that exposes the mat-core mixin. We can also keep it as is for now and import the individual typography mixins.

@devversion devversion force-pushed the density-all-mixins branch from 126c082 to cf28dc0 Compare March 9, 2020 14:14
@devversion devversion requested a review from a team as a code owner March 9, 2020 14:14
Similarly to `mdc-density`, we add a mixin for all of the
density styles that will exist in `src/material`.

Additionally, mdc-typography and mdc-theming mixins are simplified
now that the `all-theme` mixins can control individual theming system
parts. This does not work for `@angular/material` typography due to
a cyclic dependency which we could eliminate if we require people
to _explicitly_ import the file where the `mat-core` mixin originates
from.
@devversion devversion force-pushed the density-all-mixins branch from cf28dc0 to cb2b3a3 Compare March 9, 2020 16:50
Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

LGTM

@jelbourn jelbourn added lgtm action: merge The PR is ready for merge by the caretaker labels Mar 9, 2020
@jelbourn jelbourn merged commit ac08c98 into angular:density-api Mar 9, 2020
devversion added a commit that referenced this pull request Mar 12, 2020
Similarly to `mdc-density`, we add a mixin for all of the
density styles that will exist in `src/material`.

Additionally, mdc-typography and mdc-theming mixins are simplified
now that the `all-theme` mixins can control individual theming system
parts. This does not work for `@angular/material` typography due to
a cyclic dependency which we could eliminate if we require people
to _explicitly_ import the file where the `mat-core` mixin originates
from.
devversion added a commit that referenced this pull request Mar 23, 2020
Similarly to `mdc-density`, we add a mixin for all of the
density styles that will exist in `src/material`.

Additionally, mdc-typography and mdc-theming mixins are simplified
now that the `all-theme` mixins can control individual theming system
parts. This does not work for `@angular/material` typography due to
a cyclic dependency which we could eliminate if we require people
to _explicitly_ import the file where the `mat-core` mixin originates
from.
devversion added a commit that referenced this pull request Mar 31, 2020
Similarly to `mdc-density`, we add a mixin for all of the
density styles that will exist in `src/material`.

Additionally, mdc-typography and mdc-theming mixins are simplified
now that the `all-theme` mixins can control individual theming system
parts. This does not work for `@angular/material` typography due to
a cyclic dependency which we could eliminate if we require people
to _explicitly_ import the file where the `mat-core` mixin originates
from.
devversion added a commit to devversion/material2 that referenced this pull request Mar 31, 2020
Similarly to `mdc-density`, we add a mixin for all of the
density styles that will exist in `src/material`.

Additionally, mdc-typography and mdc-theming mixins are simplified
now that the `all-theme` mixins can control individual theming system
parts. This does not work for `@angular/material` typography due to
a cyclic dependency which we could eliminate if we require people
to _explicitly_ import the file where the `mat-core` mixin originates
from.
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Apr 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants