moog16
commented
5 years ago @wouldntsavezion thanks for opening the issue. You're right, this is a weak check and doesn't work in many cases as you stated above. I think removing this is an easier option. I'll have to run it by the designers as this was a design choice (just poor engineering execution).
Ripple provides a11y support, which is why it is "require". I think documenting this is an easier way to go.
(See enableRippleOnElement)
Components used in the TopAppBar need to have ripples (which is fine even though I cannot find that requirement in the MD specs). The problem is that the current check is only :
... Which doesn't in any way assure that the component actually has ripples.
Furthermore, if an element passes this check (As any Component will), the "hasRipple" prop will be set to "true", which doesn't make sense for elements that don't have such a prop in the first place. (Actually the only one I can find is Material Icon, and the readme does say that using "hasRipple" is recommended)
To add to the confusion, many elements (Like Button) actually have ripples, so, intuitively, using them shouldn't be a problem, except that doing so you end up with the following React warning :
This is because many components have "{...otherProps}" (See Here in Button). And so does the HOC Example.
I think making sure "enableRippleOnElement" doesn't needlessly inject "hasRipple:true" would be a better idea, or changing the readme to mention that "hasRipple" is required instead of recommended.