Skip to content
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

feat(notched-outline): add feature targeting for styles #5289

Conversation

crisbeto
Copy link
Collaborator

Adds support for feature targeting to the mdc-notched-outline styles.

Relates to #4227.

@codecov-io
Copy link

codecov-io commented Nov 29, 2019

Codecov Report

Merging #5289 into master will decrease coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5289      +/-   ##
==========================================
- Coverage   98.52%   98.49%   -0.04%     
==========================================
  Files         163      163              
  Lines        6309     6309              
  Branches      864      787      -77     
==========================================
- Hits         6216     6214       -2     
- Misses         93       95       +2
Impacted Files Coverage Δ
testing/dom/events.ts 71.42% <0%> (-28.58%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b240bcc...00fc001. Read the comment docs.

@crisbeto crisbeto marked this pull request as ready for review November 29, 2019 11:54
@@ -55,16 +57,20 @@
}
}

@mixin mdc-floating-label-float-position($positionY, $positionX: 0%, $scale: .75) {
@mixin mdc-floating-label-float-position($positionY, $positionX: 0%, $scale: .75, $query: mdc-feature-all()) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I copied this change from #5287 since I needed it here and I didn't want to wait for the other PR to be merged. I'll rebase if necessary.

Copy link
Collaborator

@mmalerba mmalerba left a comment

Choose a reason for hiding this comment

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

Looks good, can you provide a before and after diff of the emitted CSS?

Adds support for feature targeting to the `mdc-notched-outline` styles.

Relates to material-components#4227.
@crisbeto
Copy link
Collaborator Author

crisbeto commented Dec 6, 2019

The files are identical @mmalerba.

before.css.txt
after.css.txt

@crisbeto crisbeto force-pushed the notched-outline-feature-targeting branch from 452052c to 601344b Compare December 6, 2019 17:23
@abhiomkar
Copy link
Collaborator

I'll need to make some additional changes to our BUILD rule for Sass dependency changes and run some tests internally before I can merge this. Will do it on this week.

Thanks!

@abhiomkar
Copy link
Collaborator

Update: I manually tested all 3 PRs internally and ran a sanity test. Looks good so far!

Will update here once global tests are complete.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants