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

fix(rtl): Removed mdc-rtl-include check from mdc-rtl-reflexive mixin #5001

Merged
merged 1 commit into from
Aug 29, 2019

Conversation

abhiomkar
Copy link
Collaborator

mdc-rtl mixin already covers this condition.

@abhiomkar abhiomkar requested a review from joyzhong August 15, 2019 18:23
@abhiomkar abhiomkar force-pushed the fix/rtl_mdc_if branch 2 times, most recently from 8639524 to 41842e1 Compare August 15, 2019 21:50
@mdc-web-bot
Copy link
Collaborator

🤖 Beep boop!

Screenshot test report 🚦

3 screenshots changed from master on commit 41842e1:

Details

Copy link
Contributor

@joyzhong joyzhong left a comment

Choose a reason for hiding this comment

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

For the screenshots that test mdc-rtl-include == false: do you think we should be adding the RTL styles ourselves in the fixture SASS, so that they render properly? Otherwise, when we make future changes to button / text input, it won't be clear whether the screenshot diffs are legitimate.

@abhiomkar
Copy link
Collaborator Author

For the screenshots that test mdc-rtl-include == false: do you think we should be adding the RTL styles ourselves in the fixture SASS, so that they render properly? Otherwise, when we make future changes to button / text input, it won't be clear whether the screenshot diffs are legitimate.

I guess we wouldn't know if the test worked if we add back the RTL overrides. Left a note in the screenshot to mention that it intended behavior when RTL is turned off.

@codecov-io
Copy link

codecov-io commented Aug 28, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@76c2619). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #5001   +/-   ##
=========================================
  Coverage          ?   98.79%           
=========================================
  Files             ?      122           
  Lines             ?     5830           
  Branches          ?      763           
=========================================
  Hits              ?     5760           
  Misses            ?       69           
  Partials          ?        1

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 76c2619...fa64b4e. Read the comment docs.

@abhiomkar abhiomkar force-pushed the fix/rtl_mdc_if branch 2 times, most recently from 812730d to fa64b4e Compare August 29, 2019 20:24
@mdc-web-bot
Copy link
Collaborator

🤖 Beep boop!

Screenshot test report 🚦

3 screenshots changed from master on commit fa64b4e:

Details

@mdc-web-bot
Copy link
Collaborator

🤖 Beep boop!

Screenshot test report 🚦

3 screenshots changed from master on commit 390f1aa:

Details

@abhiomkar abhiomkar merged commit 6e7b191 into master Aug 29, 2019
@abhiomkar abhiomkar deleted the fix/rtl_mdc_if branch August 29, 2019 21:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants