When creating a DialogViewModel, an optional Boolean can be passed in to describe whether or not the positive button action is destructive or not. If set to true, the positive button color will be R.color.red. If set to false (either explicitly or by default), the color will be R.color.violet_70.
All previous usages of setting a color resource on a DialogViewModel should be changed to true (a destructive red is the only color currently being set). The actual usage of the color resource was moved to be alongside the rest of the dialog UI definitions. No other colors are currently being used for this button and the isDestructive property will default to false if not specified.
The UI should remain the same for all previously existing DialogActions.
Non-destructive dialogs (ex: SecurityDisclaimer and OnboardingSecurityDialogAutomatic) should still have the same R.color.violet_70 positive button color, and destructive dialogs (ex: DeleteConfirmationDialog and DiscardChangesDialog) should still have R.color.red for their positive buttons.
Screenshots or Videos
Here's two examples of a destructive and non-destructive dialog. Let me know if you need pictures for the rest of the dialogs.
To Do
add “WIP” to the PR title if pushing up but not complete nor ready for review
[ ] double check the original issue to confirm it is fully satisfied
[ ] add testing notes and screenshots in PR description to help guide reviewers
I did not notice any unit or instrumentation tests for DialogViewModel, DialogAction, or etcetera for the button colors, title strings and such. Are tests not needed for these files or functionality? I have not created tests yet, but will do so if needed. There were also two unit tests failing in the current master branch that I ignored for now.
Using a Boolean like this would only allow for two colors. Are there plans to add more color options, or should this be converted into something like an enum to support more colors?
Fixes #1167
Testing and Review Notes
When creating a
DialogViewModel, an optionalBooleancan be passed in to describe whether or not the positive button action is destructive or not. If set totrue, the positive button color will beR.color.red. If set tofalse(either explicitly or by default), the color will beR.color.violet_70.All previous usages of setting a color resource on a
DialogViewModelshould be changed totrue(a destructive red is the only color currently being set). The actual usage of the color resource was moved to be alongside the rest of the dialog UI definitions. No other colors are currently being used for this button and theisDestructiveproperty will default tofalseif not specified.The UI should remain the same for all previously existing
DialogActions. Non-destructive dialogs (ex:SecurityDisclaimerandOnboardingSecurityDialogAutomatic) should still have the sameR.color.violet_70positive button color, and destructive dialogs (ex:DeleteConfirmationDialogandDiscardChangesDialog) should still haveR.color.redfor their positive buttons.Screenshots or Videos
Here's two examples of a destructive and non-destructive dialog. Let me know if you need pictures for the rest of the dialogs.
To Do
Miscellaneous concerns
I did not notice any unit or instrumentation tests for
DialogViewModel,DialogAction, or etcetera for the button colors, title strings and such. Are tests not needed for these files or functionality? I have not created tests yet, but will do so if needed. There were also two unit tests failing in the current master branch that I ignored for now.Using a Boolean like this would only allow for two colors. Are there plans to add more color options, or should this be converted into something like an enum to support more colors?