fix(card): add missing hasRipple (#248)

[bmihelac]

No description provided.

[moog16]

Just curious what the use case for not having the ripple is?

[moog16]

Instead of PrimaryContentDefault, can we name it PrimaryContentBase?

[bmihelac]

No problem. That should probably be renamed as well in material-icon:

https://github.com/material-components/material-components-web-react/blob/master/packages/material-icon/index.js#L57-L80

[bmihelac]

Good question. Looking mdc-card it looks like there is no use-case for PrimaryContent without ripple:

Only applicable to cards that have a primary action that the main surface should trigger.

I can refactor this PR to remove hasRipple prop and export decorated with withRipple component instead.

[moog16]

@bmihelac that sounds good to me!

[bmihelac]

@moog16 PR has been refactored (would rebase when add tests)

One thing that I am not sure how to do is testing that PrimaryContent has ripple. Here is my not working code:

material-components-web-react/test/unit/card/Content.test.js

Lines 15 to 21 in 728a0ee

	test('it should contain mdc-ripple-upgraded', () => {
	const wrapper = mount(<CardPrimaryContent/>);
	//assert.isTrue(
	//wrapper.update()
	//.find('.mdc-card__primary-action')
	//.hasClass('mdc-ripple-upgraded'));
	});

Any idea how to test this?

[moog16]

@bmihelac figured it out...the upgraded class is added in a requestAnimationFrame. So just wrap the assert in a requestAnimationFrame.

[moog16]

@bmihelac - instead of wrapping it in a rAF, please use the rAF helper we have https://github.com/material-components/material-components-web-react/blob/master/test/unit/helpers/raf.js. Example seen in ripple unit test.

[bmihelac]

Thanks @moog16, that worked perfectly.
I have added missing test and rebased PR.

[bmihelac]

Let me know if anything else is needed regarding this.

[moog16]

looks good to me. I'm going to open another pr for tests. Thanks!

[codecov-io]

Codecov Report

Merging #255 into master will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master     #255   +/-   ##
=======================================
  Coverage   97.61%   97.61%           
=======================================
  Files          27       27           
  Lines        1089     1089           
  Branches      107      107           
=======================================
  Hits         1063     1063           
  Misses         26       26

Impacted Files	Coverage Δ
packages/card/PrimaryContent.js	100% <100%> (ø)	⬆️

---

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 731e980...aae5803. Read the comment docs.

[moog16]

Once CI passes on this PR: #263 we can merge this PR.

@bmihelac can you please sign this PR?

[bmihelac]

@bmihelac can you please sign this PR?

signed.

[moog16]

CI Passing: https://travis-ci.org/material-components/material-components-web-react/builds/430245677?utm_source=github_status&utm_medium=notification

Merging! Thanks again @bmihelac

[bmihelac]

thank you @moog16 !