Rewrite ImportDialog with Kotlin + Compose - #36
Conversation
8a62db2 to
ee09739
Compare
cbaca32 to
c9ac9d9
Compare
d91f590 to
8483f81
Compare
| @OptIn(ExperimentalMaterial3Api::class) | ||
| ModalBottomSheet( | ||
| onDismissRequest = { onAction(Action.Dismiss) }, | ||
| sheetState = rememberStandardBottomSheetState( |
There was a problem hiding this comment.
This should be rememberModalBottomSheetState(skipPartiallyExpanded = true), otherwise "back" gesture can partially collapse the sheet instead of closing it
There was a problem hiding this comment.
I actually done that in the next PRs and forgot to backport it here. Now it's actually: confirmValueChange = { it != SheetValue.PartiallyExpanded },
There was a problem hiding this comment.
It should be rememberModalBottomSheetState(skipPartiallyExpanded = true). If window exceeds half the screen, it will be undismissable by "back" otherwise.
There was a problem hiding this comment.
Sorry, I missed that rememberStandardBottomSheetState was different from rememberModalBottomSheetState 🤦
| is Effect.OpenSimImport -> { | ||
| activity.startActivity( | ||
| UIIntents.getSimImportIntent(activity, effect.subscriptionId), | ||
| ) |
There was a problem hiding this comment.
Probably need to add finish here
| bidiFormatter.unicodeWrap( | ||
| PhoneNumberUtilsCompat.createTtsSpannable(phone), | ||
| TextDirectionHeuristicsCompat.LTR, | ||
| )?.toString() |
There was a problem hiding this comment.
toString removes TTS spans. Would be better to render it as AnnotatedString with VerbatimTtsAnnotation.
| Text( | ||
| text = stringResource( | ||
| R.string.import_from_sim_summary_fmt, | ||
| option.name ?: (index + 1), |
There was a problem hiding this comment.
Old code had a special case for one sim card, it used R.string.import_from_sim there. Without it, result may look weird, for example, SIM SIM 1, SIM Verizon, etc
609c7e6 to
2838c3f
Compare
| } | ||
|
|
||
| private fun annotatedPhone(simCard: SimCard): AnnotatedString { | ||
| val phone = simCard.getFormattedPhone() ?: simCard.phone |
There was a problem hiding this comment.
simCard.phone - potential NPE below if simCard.phone is null.
|
|
||
| return when { | ||
| option.contactsCount != null && !option.phone.isNullOrBlank() -> { | ||
| expandStringTemplate( |
There was a problem hiding this comment.
TTS annotation will be still destroyed. expandStringTemplate -> TextUtils.expandTemplate -> SpannableStringBuilder.replace, it copies spans only from Spanned inputs, and AnnotatedString is not Spanned.
…on and ensure annotations are kept.
2 participants
Related with #35