Skip to content

feat: add mixins for density styles of all components #18761

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

Merged
merged 1 commit into from
Mar 9, 2020

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