Dialog is having ripple effect

[ritesh-malav]

Dialog in android is showing ripple effect. I believe this will make the change.

[codecov-io]

Codecov Report

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

@@           Coverage Diff           @@
##           master     #279   +/-   ##
=======================================
  Coverage   76.61%   76.61%           
=======================================
  Files          19       19           
  Lines         774      774           
  Branches      113      113           
=======================================
  Hits          593      593           
  Misses        150      150           
  Partials       31       31

[xotahal]

Why do you think there should be no ripple for dialog?

[ritesh-malav]

That is how the standard behaviour of dialog should be.

[xotahal]

According what? Could you send me a link to study? And how you catch onPress then?

[ritesh-malav]

Well a dialog is more of a modified version of an alert and we don't see ripple effect in android alerts.

[ritesh-malav]

an example can be whenever you visit a mail in gmail app and click on top right button and try to report it as span and dialog box will open up and that dialog box does not have ripple effect. I believe since google owns that application they would have implemented it accordingly.

[ritesh-malav]

i believe this c869739 change will take care of the onPress();

[xotahal]

View's onpress can't work i think .. and about ripple .. i believe you are able to find plenty of examples of dialogs that doesn't have onPress at all .. but at same time, you can find a dialogs that have onPress and ripple effect .. i believe that even the google has implemented that in some app ..

[ritesh-malav]

Well as of now i have not seen any app that has ripple effect on the full popup window but still if we want to keep it we can have it as props so that user can have the ripple effect if he needs it or otherwise.

[xotahal]

I think it should be prop of RippleFeedback component ..

If there is no onPress we should have avoided to render Ripple effect ...

[ritesh-malav]

No it shall not be, that way we are violating the RippleFeedback component. Use of the RippleFeedback component is to provide ripple to another component. If you remove ripple from RippleFeedback component then its like having an empty component (RippleFeedback) that makes no sense.

[ritesh-malav]

Or just make it simple, since most of the dialog box does not have ripple effect we can remove it completely. That way we don't need to handle onPress. Shall i create one pull request.

[xotahal]

But we can remove ripple effect from RippleFeedback only if there is no onPress method. Because it doesn't make sense to have an UI feedback for component that doesn't have event listener.

Then you can remove feedback effect from any component any time, only by remove onPress prop ...

[xotahal]

I am going to close this since I think Dialog can have RippleFeedback. We can remove RippleFeedback directly in RippleFeedback component if there is no onPress method.

[ritesh-malav]

Sorry for the delay, i was out, how about we pass one prop to Dialog container that will disable the native that falls under RippleFeedback container.