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

fix(ripple): Move feature detect CSS to mixins #1302

Merged
merged 3 commits into from
Sep 15, 2017
Merged

Conversation

kfranqueiro
Copy link
Contributor

This allows the feature detect CSS to be automatically included by other components
that rely on ripple mixins (e.g. button). This problem was evidenced after I split demos.scss in #1285 which caused regressions in Edge due to the feature-detect CSS no longer being loaded in other pages such as button.html. Thanks @yeelan0319 for catching this!

It may seem odd for a mixins file to emit styles, but ripple's mixins file already does so because it imports ./keyframes (which contributes to #1295). In both cases (keyframes and the feature-detect styles), this is done so that other components which are designed to support or instantiate ripples (e.g. button, dialog, etc.) function correctly without needing to separately explicitly include mdc-ripple.scss.

Kenneth G. Franqueiro and others added 2 commits September 14, 2017 14:52
This allows the feature detect CSS to be included by other components
that rely on ripple mixins (e.g. button).
@codecov-io
Copy link

codecov-io commented Sep 14, 2017

Codecov Report

Merging #1302 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1302   +/-   ##
======================================
  Coverage    99.9%   99.9%           
======================================
  Files          69      69           
  Lines        3314    3314           
  Branches      409     409           
======================================
  Hits         3311    3311           
  Misses          3       3

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 a566b81...33c77e8. Read the comment docs.

Copy link
Contributor

@lynnmercier lynnmercier left a comment

Choose a reason for hiding this comment

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

LGTM...given that we already do this for Ripple, and it fixes a regression.

But in the future we really need to revisit this Ripple code, and its relationship to other components.

@kfranqueiro kfranqueiro merged commit 628b8c4 into master Sep 15, 2017
@kfranqueiro kfranqueiro deleted the fix/demos-ripple-fd branch September 15, 2017 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants