feat: Add confirmation when trying to leave the new transfer flow #255
Conversation
LunarX
force-pushed
the
mail-validation-ui
branch
from
4fe805d to
2f92daf
Compare
LunarX
force-pushed
the
ask-user-before-leaving-new-transfer
branch
2 times, most recently
from
f8b8d2c to
87feb4a
Compare
LunarX
force-pushed
the
mail-validation-ui
branch
from
2f92daf to
b9cff14
Compare
|
This PR/issue depends on:
|
LunarX
force-pushed
the
ask-user-before-leaving-new-transfer
branch
4 times, most recently
from
becbb9b to
f914928
Compare
LunarX
force-pushed
the
ask-user-before-leaving-new-transfer
branch
5 times, most recently
from
a2fd0bf to
d270eac
Compare
| @@ -107,6 +108,7 @@ private fun CoreButton( | |||
| showIndeterminateProgress, | |||
| progress, | |||
| onClick, | |||
| contentPadding = buttonSize.contentPadding | |||
There was a problem hiding this comment.
| contentPadding = buttonSize.contentPadding | |
| contentPadding = buttonSize.contentPadding, |
app/src/main/java/com/infomaniak/swisstransfer/ui/components/SwissTransferAlertDialog.kt
Show resolved
Hide resolved
| fun confirmButton(isEnabled: () -> Boolean = { true }, onClick: () -> Unit) { | ||
| SmallButton( | ||
| style = ButtonType.TERTIARY, | ||
| title = stringResource(R.string.buttonConfirm), |
There was a problem hiding this comment.
We could take this string to Core2 exaclty as the buttonCancel
| @@ -31,15 +31,36 @@ import com.infomaniak.swisstransfer.ui.theme.Margin | |||
| import com.infomaniak.swisstransfer.ui.theme.SwissTransferTheme | |||
| import com.infomaniak.core2.R as RCore2 | |||
|
|
|||
| object SwissTransferAlertDialogDefaults { | |||
There was a problem hiding this comment.
This is nice ! Adds real material component logic to this custom one
| @@ -124,6 +124,12 @@ enum class ButtonType(val buttonColors: @Composable () -> ButtonColors) { | |||
| contentColor = SwissTransferTheme.materialColors.onError, | |||
| ) | |||
| }), | |||
| ERROR_TEXT({ | |||
There was a problem hiding this comment.
This is not really an error text, more of a destructive action button
| @@ -97,6 +98,8 @@ fun ImportFilesScreen( | |||
|
|
|||
| LaunchedEffect(Unit) { importFilesViewModel.initTransferOptionsValues() } | |||
|
|
|||
| BackHandler { closeActivity() } // Closing the activity like other screens will prompt the user to be sure he wants to leave | |||
There was a problem hiding this comment.
I was a bit confuse with this sentence at first
Maybe we could rewrite this along the line of "We need to overwrite this to avoid prompting the user to be sure he wants to leave" ?
| positiveButton: @Composable () -> Unit, | ||
| negativeButton: @Composable () -> Unit, |
There was a problem hiding this comment.
This should already use by default your newly created default buttons
I'd suggest to make this nullable all the way to ActionButton, and then manage it there
There was a problem hiding this comment.
I would've loved to as well, but we need to define the on click callbacks outside of it, so I can't set them as default values. The only option would be to provide two signatures for the SwissTransferAlertDialog; one that takes composables and one that takes only onClick callbacks. But this feels a bit too much :/
LunarX
force-pushed
the
ask-user-before-leaving-new-transfer
branch
from
d270eac to
109e6b5
Compare
LunarX
force-pushed
the
mail-validation-ui
branch
from
8cde32f to
3084096
Compare
Don't let them escape!
When canceling an upload in progress, you'll be asked to confirm a first time and then when you're back on ImportFilesScreen, you'll need to confirm again to leave. This takes way too many clicks, the confirmation on ImportFilesScreen is not needed
LunarX
force-pushed
the
ask-user-before-leaving-new-transfer
branch
from
109e6b5 to
9b62657
Compare
|
| } | ||
|
|
||
| @Composable | ||
| fun cancelButton(onClick: () -> Unit) { |
There was a problem hiding this comment.
This should be Uppercase
LunarX
force-pushed
the
mail-validation-ui
branch
from
40e505e to
6ecf5a5
Compare
Don't let them escape!
This required me to refactor the dialog composable we already have to have greater control over the buttons. I opted for a more standard interfacing logic instead of adding a lot of parameters. We can now compose the dialog with buttons. There's still two constant buttons already defined for the standard cases.
This PR also required to add stacking buttons in dialogs when the buttons are too wide which required to change the design of the buttons for plain text buttons like the native material theme so they are not misaligned when stacked.
Depends on #254