adding a custom dialog #74
Conversation
| public void setCustomDialog(@NonNull DialogFragment fragment) { | ||
| customFragment = fragment; |
There was a problem hiding this comment.
wouldn't it make more sense to add this field to the ShakeDelegate? It's giving me the vibe that communicating little customizations like this is the purpose of the delegate.
There was a problem hiding this comment.
So if you look at the current flow altogether, you can only alter the dialog title and message and not the button text or behavior. And also, the delegate starts the feedback flow whereas we are adding a feature that could be run before the feedback flow if a custom dialog is provided
| sendFeedbackDialog.show(activity.getFragmentManager(), SEND_FEEDBACK_TAG); | ||
| if (customFragment != null) { | ||
| customFragment.show(activity.getFragmentManager(), CUSTOM_DIALOG_TAG); |
There was a problem hiding this comment.
tiny nit - it seems like a lot of the logic above this line could probably go in the else block since we're not actually doing anything with the sendFeedbackDialog right? And then similar question for the shakyFlowCallback on 171 and on 197.
There was a problem hiding this comment.
I kept the code as close to existing code as possible and i feel a more condensed if else loop would increase readability and understanding of the logic a lot easier. If it's not a logic breaking change and just for aesthetics, I really appreciate the suggestion but would like to keep the code this way so the PR is short too and easy to track down for other developers
Adding a custom dialog to be shown before the feedback flow dialog fragment