Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create room : improve handling of room address #3868

Merged
merged 3 commits into from
Nov 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,4 @@ data class CreateRoomConfig(
val avatarUri: Uri? = null,
val invites: ImmutableList<MatrixUser> = persistentListOf(),
val roomVisibility: RoomVisibilityState = RoomVisibilityState.Private,
) {
val isValid = roomName.isNullOrEmpty().not() && roomVisibility.isValid()
}
)
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@
import io.element.android.features.createroom.impl.configureroom.RoomAccess
import io.element.android.features.createroom.impl.configureroom.RoomAccessItem
import io.element.android.features.createroom.impl.configureroom.RoomAddress
import io.element.android.features.createroom.impl.configureroom.RoomAddressErrorState
import io.element.android.features.createroom.impl.configureroom.RoomVisibilityItem
import io.element.android.features.createroom.impl.configureroom.RoomVisibilityState
import io.element.android.features.createroom.impl.di.CreateRoomScope
import io.element.android.features.createroom.impl.userlist.UserListDataStore
import io.element.android.libraries.androidutils.file.safeDelete
import io.element.android.libraries.di.SingleIn
import io.element.android.libraries.matrix.api.room.alias.RoomAliasHelper
import kotlinx.collections.immutable.toImmutableList
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableStateFlow
Expand All @@ -29,6 +29,7 @@
@SingleIn(CreateRoomScope::class)
class CreateRoomDataStore @Inject constructor(
val selectedUserListDataStore: UserListDataStore,
private val roomAliasHelper: RoomAliasHelper,
) {
private val createRoomConfigFlow: MutableStateFlow<CreateRoomConfig> = MutableStateFlow(CreateRoomConfig())
private var cachedAvatarUri: Uri? = null
Expand All @@ -46,23 +47,23 @@

fun setRoomName(roomName: String) {
createRoomConfigFlow.getAndUpdate { config ->
/*
val newVisibility = when (config.roomVisibility) {
is RoomVisibilityState.Public -> {
val roomAddress = config.roomVisibility.roomAddress
if (roomAddress is RoomAddress.AutoFilled || roomName.isEmpty()) {
val roomAliasName = roomAliasHelper.roomAliasNameFromRoomDisplayName(roomName)

Check warning on line 54 in features/createroom/impl/src/main/kotlin/io/element/android/features/createroom/impl/CreateRoomDataStore.kt

View check run for this annotation

Codecov / codecov/patch

features/createroom/impl/src/main/kotlin/io/element/android/features/createroom/impl/CreateRoomDataStore.kt#L54

Added line #L54 was not covered by tests
config.roomVisibility.copy(
roomAddress = RoomAddress.AutoFilled(roomName),
roomAddress = RoomAddress.AutoFilled(roomAliasName),

Check warning on line 56 in features/createroom/impl/src/main/kotlin/io/element/android/features/createroom/impl/CreateRoomDataStore.kt

View check run for this annotation

Codecov / codecov/patch

features/createroom/impl/src/main/kotlin/io/element/android/features/createroom/impl/CreateRoomDataStore.kt#L56

Added line #L56 was not covered by tests
)
} else {
config.roomVisibility
}
}
else -> config.roomVisibility
}
*/
config.copy(
roomName = roomName.takeIf { it.isNotEmpty() },
roomVisibility = newVisibility,
)
}
}
Expand All @@ -85,11 +86,13 @@
config.copy(
roomVisibility = when (visibility) {
RoomVisibilityItem.Private -> RoomVisibilityState.Private
RoomVisibilityItem.Public -> RoomVisibilityState.Public(
roomAddress = RoomAddress.AutoFilled(config.roomName.orEmpty()),
roomAddressErrorState = RoomAddressErrorState.None,
roomAccess = RoomAccess.Anyone,
)
RoomVisibilityItem.Public -> {
val roomAliasName = roomAliasHelper.roomAliasNameFromRoomDisplayName(config.roomName.orEmpty())
RoomVisibilityState.Public(
roomAddress = RoomAddress.AutoFilled(roomAliasName),
roomAccess = RoomAccess.Anyone,
)
}
}
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.rememberCoroutineScope
import androidx.compose.runtime.rememberUpdatedState
import im.vector.app.features.analytics.plan.CreatedRoom
import io.element.android.features.createroom.impl.CreateRoomConfig
import io.element.android.features.createroom.impl.CreateRoomDataStore
Expand All @@ -31,6 +32,8 @@
import io.element.android.libraries.matrix.api.createroom.CreateRoomParameters
import io.element.android.libraries.matrix.api.createroom.RoomPreset
import io.element.android.libraries.matrix.api.createroom.RoomVisibility
import io.element.android.libraries.matrix.api.room.alias.RoomAliasHelper
import io.element.android.libraries.matrix.api.roomAliasFromName
import io.element.android.libraries.matrix.ui.media.AvatarAction
import io.element.android.libraries.mediapickers.api.PickerProvider
import io.element.android.libraries.mediaupload.api.MediaPreProcessor
Expand All @@ -39,9 +42,12 @@
import io.element.android.services.analytics.api.AnalyticsService
import kotlinx.collections.immutable.toImmutableList
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.delay
import kotlinx.coroutines.launch
import timber.log.Timber
import java.util.Optional
import javax.inject.Inject
import kotlin.jvm.optionals.getOrNull

class ConfigureRoomPresenter @Inject constructor(
private val dataStore: CreateRoomDataStore,
Expand All @@ -51,16 +57,20 @@
private val analyticsService: AnalyticsService,
permissionsPresenterFactory: PermissionsPresenter.Factory,
private val featureFlagService: FeatureFlagService,
private val roomAliasHelper: RoomAliasHelper,
) : Presenter<ConfigureRoomState> {
private val cameraPermissionPresenter: PermissionsPresenter = permissionsPresenterFactory.create(android.Manifest.permission.CAMERA)
private var pendingPermissionRequest = false

@Composable
override fun present(): ConfigureRoomState {
val cameraPermissionState = cameraPermissionPresenter.present()
val createRoomConfig = dataStore.createRoomConfigWithInvites.collectAsState(CreateRoomConfig())
val createRoomConfig by dataStore.createRoomConfigWithInvites.collectAsState(CreateRoomConfig())
val homeserverName = remember { matrixClient.userIdServerName() }
val isKnockFeatureEnabled by featureFlagService.isFeatureEnabledFlow(FeatureFlags.Knock).collectAsState(initial = false)
val roomAddressValidity = remember {
mutableStateOf<RoomAddressValidity>(RoomAddressValidity.Unknown)
}

val cameraPhotoPicker = mediaPickerProvider.registerCameraPhotoPicker(
onResult = { uri -> if (uri != null) dataStore.setAvatarUri(uri = uri, cached = true) },
Expand All @@ -69,12 +79,12 @@
onResult = { uri -> if (uri != null) dataStore.setAvatarUri(uri = uri) }
)

val avatarActions by remember(createRoomConfig.value.avatarUri) {
val avatarActions by remember(createRoomConfig.avatarUri) {
derivedStateOf {
listOfNotNull(
AvatarAction.TakePhoto,
AvatarAction.ChoosePhoto,
AvatarAction.Remove.takeIf { createRoomConfig.value.avatarUri != null },
AvatarAction.Remove.takeIf { createRoomConfig.avatarUri != null },
).toImmutableList()
}
}
Expand All @@ -86,6 +96,10 @@
}
}

RoomAddressValidityEffect(createRoomConfig.roomVisibility.roomAddress()) { newRoomAddressValidity ->
roomAddressValidity.value = newRoomAddressValidity
}

val localCoroutineScope = rememberCoroutineScope()
val createRoomAction: MutableState<AsyncAction<RoomId>> = remember { mutableStateOf(AsyncAction.Uninitialized) }

Expand All @@ -102,7 +116,7 @@
is ConfigureRoomEvents.RemoveUserFromSelection -> dataStore.selectedUserListDataStore.removeUserFromSelection(event.matrixUser)
is ConfigureRoomEvents.RoomAccessChanged -> dataStore.setRoomAccess(event.roomAccess)
is ConfigureRoomEvents.RoomAddressChanged -> dataStore.setRoomAddress(event.roomAddress)
is ConfigureRoomEvents.CreateRoom -> createRoom(createRoomConfig.value)
is ConfigureRoomEvents.CreateRoom -> createRoom(createRoomConfig)
is ConfigureRoomEvents.HandleAvatarAction -> {
when (event.action) {
AvatarAction.ChoosePhoto -> galleryImagePicker.launch()
Expand All @@ -122,15 +136,49 @@

return ConfigureRoomState(
isKnockFeatureEnabled = isKnockFeatureEnabled,
config = createRoomConfig.value,
config = createRoomConfig,
avatarActions = avatarActions,
createRoomAction = createRoomAction.value,
cameraPermissionState = cameraPermissionState,
homeserverName = homeserverName,
roomAddressValidity = roomAddressValidity.value,
eventSink = ::handleEvents,
)
}

@Composable
private fun RoomAddressValidityEffect(
roomAddress: Optional<String>,
onRoomAddressValidityChange: (RoomAddressValidity) -> Unit,
) {
val onChange by rememberUpdatedState(onRoomAddressValidityChange)
LaunchedEffect(roomAddress) {
val roomAliasName = roomAddress.getOrNull().orEmpty()
if (roomAliasName.isEmpty()) {
onChange(RoomAddressValidity.Unknown)
return@LaunchedEffect
}
// debounce the room address validation
delay(300)
val roomAlias = matrixClient.roomAliasFromName(roomAliasName).getOrNull()
if (roomAlias == null || !roomAliasHelper.isRoomAliasValid(roomAlias)) {
onChange(RoomAddressValidity.InvalidSymbols)
} else {
matrixClient.resolveRoomAlias(roomAlias)
.onSuccess { resolved ->
if (resolved.isPresent) {
onChange(RoomAddressValidity.NotAvailable)
} else {
onChange(RoomAddressValidity.Valid)
}
}
.onFailure {
onChange(RoomAddressValidity.Valid)
}

Check warning on line 177 in features/createroom/impl/src/main/kotlin/io/element/android/features/createroom/impl/configureroom/ConfigureRoomPresenter.kt

View check run for this annotation

Codecov / codecov/patch

features/createroom/impl/src/main/kotlin/io/element/android/features/createroom/impl/configureroom/ConfigureRoomPresenter.kt#L176-L177

Added lines #L176 - L177 were not covered by tests
}
}
}

private fun CoroutineScope.createRoom(
config: CreateRoomConfig,
createRoomAction: MutableState<AsyncAction<RoomId>>
Expand All @@ -148,7 +196,7 @@
preset = RoomPreset.PUBLIC_CHAT,
invite = config.invites.map { it.userId },
avatar = avatarUrl,
canonicalAlias = config.roomVisibility.roomAddress()
roomAliasName = config.roomVisibility.roomAddress()

Check warning on line 199 in features/createroom/impl/src/main/kotlin/io/element/android/features/createroom/impl/configureroom/ConfigureRoomPresenter.kt

View check run for this annotation

Codecov / codecov/patch

features/createroom/impl/src/main/kotlin/io/element/android/features/createroom/impl/configureroom/ConfigureRoomPresenter.kt#L199

Added line #L199 was not covered by tests
)
} else {
CreateRoomParameters(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ data class ConfigureRoomState(
val avatarActions: ImmutableList<AvatarAction>,
val createRoomAction: AsyncAction<RoomId>,
val cameraPermissionState: PermissionsState,
val roomAddressValidity: RoomAddressValidity,
val homeserverName: String,
val eventSink: (ConfigureRoomEvents) -> Unit
)
) {
val isValid: Boolean = config.roomName?.isNotEmpty() == true &&
(config.roomVisibility is RoomVisibilityState.Private || roomAddressValidity == RoomAddressValidity.Valid)
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,8 @@ open class ConfigureRoomStateProvider : PreviewParameterProvider<ConfigureRoomSt
topic = "Room topic for this room when the text goes onto multiple lines and is really long, there shouldn’t be more than 3 lines",
invites = aMatrixUserList().toImmutableList(),
roomVisibility = RoomVisibilityState.Public(
roomAddress = RoomAddress.AutoFilled("Room 101"),
roomAddress = RoomAddress.AutoFilled("Room-101"),
roomAccess = RoomAccess.Knocking,
roomAddressErrorState = RoomAddressErrorState.None,
),
),
),
Expand All @@ -40,12 +39,44 @@ open class ConfigureRoomStateProvider : PreviewParameterProvider<ConfigureRoomSt
topic = "Room topic for this room when the text goes onto multiple lines and is really long, there shouldn’t be more than 3 lines",
invites = aMatrixUserList().toImmutableList(),
roomVisibility = RoomVisibilityState.Public(
roomAddress = RoomAddress.AutoFilled("Room 101"),
roomAddress = RoomAddress.AutoFilled("Room-101"),
roomAccess = RoomAccess.Knocking,
roomAddressErrorState = RoomAddressErrorState.None,
),
),
),
aConfigureRoomState(
config = CreateRoomConfig(
roomName = "Room 101",
topic = "Room topic for this room when the text goes onto multiple lines and is really long, there shouldn’t be more than 3 lines",
roomVisibility = RoomVisibilityState.Public(
roomAddress = RoomAddress.AutoFilled("Room-101"),
roomAccess = RoomAccess.Knocking,
),
),
roomAddressValidity = RoomAddressValidity.NotAvailable,
),
aConfigureRoomState(
config = CreateRoomConfig(
roomName = "Room 101",
topic = "Room topic for this room when the text goes onto multiple lines and is really long, there shouldn’t be more than 3 lines",
roomVisibility = RoomVisibilityState.Public(
roomAddress = RoomAddress.AutoFilled("Room-101"),
roomAccess = RoomAccess.Knocking,
),
),
roomAddressValidity = RoomAddressValidity.InvalidSymbols,
),
aConfigureRoomState(
config = CreateRoomConfig(
roomName = "Room 101",
topic = "Room topic for this room when the text goes onto multiple lines and is really long, there shouldn’t be more than 3 lines",
roomVisibility = RoomVisibilityState.Public(
roomAddress = RoomAddress.AutoFilled("Room-101"),
roomAccess = RoomAccess.Knocking,
),
),
roomAddressValidity = RoomAddressValidity.Valid,
),
)
}

Expand All @@ -56,6 +87,7 @@ fun aConfigureRoomState(
createRoomAction: AsyncAction<RoomId> = AsyncAction.Uninitialized,
cameraPermissionState: PermissionsState = aPermissionsState(showDialog = false),
homeserverName: String = "matrix.org",
roomAddressValidity: RoomAddressValidity = RoomAddressValidity.Valid,
eventSink: (ConfigureRoomEvents) -> Unit = { },
) = ConfigureRoomState(
config = config,
Expand All @@ -64,5 +96,6 @@ fun aConfigureRoomState(
createRoomAction = createRoomAction,
cameraPermissionState = cameraPermissionState,
homeserverName = homeserverName,
roomAddressValidity = roomAddressValidity,
eventSink = eventSink,
)
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.ColumnScope
import androidx.compose.foundation.layout.PaddingValues
import androidx.compose.foundation.layout.Row
import androidx.compose.foundation.layout.Spacer
import androidx.compose.foundation.layout.consumeWindowInsets
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.imePadding
Expand Down Expand Up @@ -79,7 +80,7 @@ fun ConfigureRoomView(
modifier = modifier.clearFocusOnTap(focusManager),
topBar = {
ConfigureRoomToolbar(
isNextActionEnabled = state.config.isValid,
isNextActionEnabled = state.isValid,
onBackClick = onBackClick,
onNextClick = {
focusManager.clearFocus()
Expand Down Expand Up @@ -143,8 +144,10 @@ fun ConfigureRoomView(
modifier = Modifier.padding(horizontal = 16.dp),
address = state.config.roomVisibility.roomAddress,
homeserverName = state.homeserverName,
addressValidity = state.roomAddressValidity,
onAddressChange = { state.eventSink(ConfigureRoomEvents.RoomAddressChanged(it)) },
)
Spacer(Modifier)
}
}
}
Expand Down Expand Up @@ -319,6 +322,7 @@ private fun RoomAccessOptions(
private fun RoomAddressField(
address: RoomAddress,
homeserverName: String,
addressValidity: RoomAddressValidity,
onAddressChange: (String) -> Unit,
modifier: Modifier = Modifier,
) {
Expand All @@ -340,7 +344,16 @@ private fun RoomAddressField(
color = ElementTheme.colors.textSecondary,
)
},
supportingText = stringResource(R.string.screen_create_room_room_address_section_footer),
supportingText = when (addressValidity) {
RoomAddressValidity.InvalidSymbols -> {
stringResource(R.string.screen_create_room_room_address_invalid_symbols_error_description)
}
RoomAddressValidity.NotAvailable -> {
stringResource(R.string.screen_create_room_room_address_not_available_error_description)
}
else -> stringResource(R.string.screen_create_room_room_address_section_footer)
},
isError = addressValidity.isError(),
onValueChange = onAddressChange,
singleLine = true,
)
Expand Down

This file was deleted.

Loading
Loading