Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

fix(typography): Separate @material/feature-targeting #5634

Merged
merged 1 commit into from
Feb 20, 2020
Merged

fix(typography): Separate @material/feature-targeting #5634

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
51 changes: 26 additions & 25 deletions packages/mdc-typography/_mixins.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,12 @@
@use "sass:list";
@use "sass:map";
@use "sass:string";
@use "@material/feature-targeting" as feature-targeting;
EstebanG23 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

What about as ft to its clearly different than feature-targeting?

I'm trying to find ways we can take advantage of the _index.scss files.

Copy link
Contributor

Choose a reason for hiding this comment

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

If we're just looking for another name, I'd recommend feature as a name that is concise and makes sense.

@use "@material/feature-targeting" as feature;

@mixin my-component-core-styles($query: feature.all()) {
  $feat-structure: feature.create-target($query, structure);

  @include feature.targets($feat-structure) {
    // ...
  }
}

Copy link
Contributor Author

@EstebanG23 EstebanG23 Feb 20, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I tried both ft and without the as feature-targeting results are as follow:

Error: Can't find stylesheet to import.
26 │ @use "@material/feature-targeting" as ft;

Also do want to add this is blocking other PRs from being merged but I would encourage to file a bug/issue to investigate ways to take advantage of the _index.scss files.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks @EstebanG23 for sending a fix! Wondering why Copybara didn't block CL submit when github action failed in first place. 🤔

Sass couldn't compile Sass test files with @import statements since the custom importer doesn't handle new _index files (See testing/featuretargeting/sass-test-compile.helper.ts).

Next steps:

@use "@material/theme";
@use "@material/feature-targeting/functions" as feature-targeting-functions;
@use "@material/feature-targeting/mixins" as feature-targeting-mixins;
@use "@material/theme/mixins" as theme-mixins;
@use "./variables";

@mixin core-styles($query: feature-targeting.all()) {
@mixin core-styles($query: feature-targeting-functions.all()) {
.mdc-typography {
@include base($query: $query);
}
Expand All @@ -39,28 +40,28 @@
}
}

@mixin base($query: feature-targeting.all()) {
$feat-typography: feature-targeting.create-target($query, typography);
@mixin base($query: feature-targeting-functions.all()) {
$feat-typography: feature-targeting-functions.create-target($query, typography);

@include smooth-font($query: $query);
@include feature-targeting.targets($feat-typography) {
@include theme.prop(font-family, (
@include feature-targeting-mixins.targets($feat-typography) {
@include theme-mixins.prop(font-family, (
varname: --mdc-typography-font-family,
fallback: map.get(variables.$base, 'font-family')
));
}
}

@mixin typography($style, $query: feature-targeting.all(), $exclude-props: ()) {
$feat-typography: feature-targeting.create-target($query, typography);
@mixin typography($style, $query: feature-targeting-functions.all(), $exclude-props: ()) {
$feat-typography: feature-targeting-functions.create-target($query, typography);
$style-props: map.get(variables.$styles, $style);

@if not map.has-key(variables.$styles, $style) {
@error "Invalid style specified! #{$style} doesn't exist. Choose one of #{map.keys(variables.$styles)}";
}

@include smooth-font($query: $query);
@include feature-targeting.targets($feat-typography) {
@include feature-targeting-mixins.targets($feat-typography) {
@each $key, $value in $style-props {
@if list.index($exclude-props, $key) == null {
$fallback: $value;
Expand All @@ -74,7 +75,7 @@
);
}

@include theme.prop($key, (
@include theme-mixins.prop($key, (
varname: --mdc-typography-#{$style}-#{$key},
fallback: $fallback
));
Expand All @@ -84,54 +85,54 @@
}

/// Applies antialiasing via font-smoothing to text.
@mixin smooth-font($query: feature-targeting.all()) {
$feat-typography: feature-targeting.create-target($query, typography);
@mixin smooth-font($query: feature-targeting-functions.all()) {
$feat-typography: feature-targeting-functions.create-target($query, typography);

@include feature-targeting.targets($feat-typography) {
@include feature-targeting-mixins.targets($feat-typography) {
-moz-osx-font-smoothing: grayscale;
-webkit-font-smoothing: antialiased;
}
}

// Element must be `display: block` or `display: inline-block` for this to work.
@mixin overflow-ellipsis($query: feature-targeting.all()) {
$feat-structure: feature-targeting.create-target($query, structure);
@mixin overflow-ellipsis($query: feature-targeting-functions.all()) {
$feat-structure: feature-targeting-functions.create-target($query, structure);

@include feature-targeting.targets($feat-structure) {
@include feature-targeting-mixins.targets($feat-structure) {
text-overflow: ellipsis;
white-space: nowrap;
overflow: hidden;
}
}

@mixin baseline-top($distance, $query: feature-targeting.all()) {
$feat-structure: feature-targeting.create-target($query, structure);
@mixin baseline-top($distance, $query: feature-targeting-functions.all()) {
$feat-structure: feature-targeting-functions.create-target($query, structure);

@include feature-targeting.targets($feat-structure) {
@include feature-targeting-mixins.targets($feat-structure) {
display: block;
margin-top: 0;
/* @alternate */
line-height: normal;
}

&::before {
@include feature-targeting.targets($feat-structure) {
@include feature-targeting-mixins.targets($feat-structure) {
@include baseline-strut_($distance);

vertical-align: 0;
}
}
}

@mixin baseline-bottom($distance, $query: feature-targeting.all()) {
$feat-structure: feature-targeting.create-target($query, structure);
@mixin baseline-bottom($distance, $query: feature-targeting-functions.all()) {
$feat-structure: feature-targeting-functions.create-target($query, structure);

@include feature-targeting.targets($feat-structure) {
@include feature-targeting-mixins.targets($feat-structure) {
margin-bottom: -1 * $distance;
}

&::after {
@include feature-targeting.targets($feat-structure) {
@include feature-targeting-mixins.targets($feat-structure) {
@include baseline-strut_($distance);

vertical-align: -1 * $distance;
Expand Down