From a1aad5376935ae1983b5f917aac4734dd5341299 Mon Sep 17 00:00:00 2001 From: Valere Date: Wed, 9 Jun 2021 13:00:50 +0200 Subject: [PATCH 1/5] Space top level ordering --- build.gradle | 3 + .../org/matrix/android/sdk/SpaceOrderTest.kt | 269 ++++++++++++++++++ .../org/matrix/android/sdk/StringOrderTest.kt | 110 +++++++ .../matrix/android/sdk/api/MatrixPatterns.kt | 11 + .../room/accountdata/RoomAccountDataTypes.kt | 1 + .../sdk/api/session/space/SpaceOrderUtils.kt | 92 ++++++ .../session/space/model/SpaceOrderContent.kt | 37 +++ .../space/model/TopLevelSpaceComparator.kt | 44 +++ .../android/sdk/api/util/StringOrderUtils.kt | 87 ++++++ .../DefaultRoomAccountDataService.kt | 4 +- vector/build.gradle | 1 + .../app/features/spaces/SpaceListAction.kt | 3 + .../app/features/spaces/SpaceListFragment.kt | 38 +++ .../app/features/spaces/SpaceListViewState.kt | 4 +- .../features/spaces/SpaceSummaryController.kt | 14 +- .../app/features/spaces/SpaceSummaryItem.kt | 1 + .../features/spaces/SpacesListViewModel.kt | 137 +++++++-- 17 files changed, 818 insertions(+), 38 deletions(-) create mode 100644 matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/SpaceOrderTest.kt create mode 100644 matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/StringOrderTest.kt create mode 100644 matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/space/SpaceOrderUtils.kt create mode 100644 matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/space/model/SpaceOrderContent.kt create mode 100644 matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/space/model/TopLevelSpaceComparator.kt create mode 100644 matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/util/StringOrderUtils.kt diff --git a/build.gradle b/build.gradle index b2130664233..92dedbe1af4 100644 --- a/build.gradle +++ b/build.gradle @@ -10,6 +10,9 @@ buildscript { maven { url "https://plugins.gradle.org/m2/" } +// flatDir { +// dirs 'libs' +// } } dependencies { classpath 'com.android.tools.build:gradle:4.2.1' diff --git a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/SpaceOrderTest.kt b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/SpaceOrderTest.kt new file mode 100644 index 00000000000..e8fdf5472ae --- /dev/null +++ b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/SpaceOrderTest.kt @@ -0,0 +1,269 @@ +/* + * Copyright 2021 The Matrix.org Foundation C.I.C. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.matrix.android.sdk + +import org.amshove.kluent.internal.assertEquals +import org.junit.Assert +import org.junit.Test +import org.matrix.android.sdk.api.session.space.SpaceOrderUtils + +class SpaceOrderTest { + + @Test + fun testOrderBetweenNodesWithOrder() { + val orderedSpaces = listOf( + "roomId1" to "a", + "roomId2" to "m", + "roomId3" to "z" + ).assertSpaceOrdered() + + val orderCommand = SpaceOrderUtils.orderCommandsForMove(orderedSpaces, "roomId1", 1) + + Assert.assertTrue("Only one order should be changed", orderCommand.size == 1) + Assert.assertTrue("Moved space order should change", orderCommand.first().spaceId == "roomId1") + + Assert.assertTrue("m" < orderCommand[0].order) + Assert.assertTrue(orderCommand[0].order < "z") + } + + @Test + fun testMoveLastBetweenNodesWithOrder() { + val orderedSpaces = listOf( + "roomId1" to "a", + "roomId2" to "m", + "roomId3" to "z" + ).assertSpaceOrdered() + + val orderCommand = SpaceOrderUtils.orderCommandsForMove(orderedSpaces, "roomId1", 2) + + Assert.assertTrue("Only one order should be changed", orderCommand.size == 1) + Assert.assertTrue("Moved space order should change", orderCommand.first().spaceId == "roomId1") + + Assert.assertTrue("z" < orderCommand[0].order) + } + + @Test + fun testMoveUpNoOrder() { + val orderedSpaces = listOf( + "roomId1" to null, + "roomId2" to null, + "roomId3" to null + ).assertSpaceOrdered() + + val orderCommand = SpaceOrderUtils.orderCommandsForMove(orderedSpaces, "roomId1", 1) + + Assert.assertTrue("2 orders change should be needed", orderCommand.size == 2) + + val reOrdered = reOrderWithCommands(orderedSpaces, orderCommand) + + Assert.assertEquals("roomId2", reOrdered[0].first) + Assert.assertEquals("roomId1", reOrdered[1].first) + Assert.assertEquals("roomId3", reOrdered[2].first) + } + + @Test + fun testMoveUpNotEnoughSpace() { + val orderedSpaces = listOf( + "roomId1" to "a", + "roomId2" to "j", + "roomId3" to "k" + ).assertSpaceOrdered() + + val orderCommand = SpaceOrderUtils.orderCommandsForMove(orderedSpaces, "roomId1", 1) + + Assert.assertTrue("more orders change should be needed", orderCommand.size > 1) + + val reOrdered = reOrderWithCommands(orderedSpaces, orderCommand) + + Assert.assertEquals("roomId2", reOrdered[0].first) + Assert.assertEquals("roomId1", reOrdered[1].first) + Assert.assertEquals("roomId3", reOrdered[2].first) + } + + @Test + fun testMoveEndNoOrder() { + val orderedSpaces = listOf( + "roomId1" to null, + "roomId2" to null, + "roomId3" to null + ).assertSpaceOrdered() + + val orderCommand = SpaceOrderUtils.orderCommandsForMove(orderedSpaces, "roomId1", 2) + + // Actually 2 could be enough... as it's last it can stays with null + Assert.assertEquals("3 orders change should be needed", 3, orderCommand.size) + + val reOrdered = reOrderWithCommands(orderedSpaces, orderCommand) + + Assert.assertEquals("roomId2", reOrdered[0].first) + Assert.assertEquals("roomId3", reOrdered[1].first) + Assert.assertEquals("roomId1", reOrdered[2].first) + } + + @Test + fun testMoveUpBiggerOrder() { + val orderedSpaces = listOf( + "roomId1" to "aaaa", + "roomId2" to "ffff", + "roomId3" to "pppp", + "roomId4" to null, + "roomId5" to null, + "roomId6" to null + ).assertSpaceOrdered() + + val orderCommand = SpaceOrderUtils.orderCommandsForMove(orderedSpaces, "roomId2", 3) + + // Actually 2 could be enough... as it's last it can stays with null + Assert.assertEquals("3 orders change should be needed", 3, orderCommand.size) + + val reOrdered = reOrderWithCommands(orderedSpaces, orderCommand) + + Assert.assertEquals("roomId1", reOrdered[0].first) + Assert.assertEquals("roomId3", reOrdered[1].first) + Assert.assertEquals("roomId4", reOrdered[2].first) + Assert.assertEquals("roomId5", reOrdered[3].first) + Assert.assertEquals("roomId2", reOrdered[4].first) + Assert.assertEquals("roomId6", reOrdered[5].first) + } + + @Test + fun testMoveDownBetweenNodesWithOrder() { + val orderedSpaces = listOf( + "roomId1" to "a", + "roomId2" to "m", + "roomId3" to "z" + ).assertSpaceOrdered() + + val orderCommand = SpaceOrderUtils.orderCommandsForMove(orderedSpaces, "roomId3", -1) + + Assert.assertTrue("Only one order should be changed", orderCommand.size == 1) + Assert.assertTrue("Moved space order should change", orderCommand.first().spaceId == "roomId3") + + val reOrdered = reOrderWithCommands(orderedSpaces, orderCommand) + + Assert.assertEquals("roomId1", reOrdered[0].first) + Assert.assertEquals("roomId3", reOrdered[1].first) + Assert.assertEquals("roomId2", reOrdered[2].first) + } + + @Test + fun testMoveDownNoOrder() { + val orderedSpaces = listOf( + "roomId1" to null, + "roomId2" to null, + "roomId3" to null + ).assertSpaceOrdered() + + val orderCommand = SpaceOrderUtils.orderCommandsForMove(orderedSpaces, "roomId3", -1) + + Assert.assertTrue("2 orders change should be needed", orderCommand.size == 2) + + val reOrdered = reOrderWithCommands(orderedSpaces, orderCommand) + + Assert.assertEquals("roomId1", reOrdered[0].first) + Assert.assertEquals("roomId3", reOrdered[1].first) + Assert.assertEquals("roomId2", reOrdered[2].first) + } + + @Test + fun testMoveDownBiggerOrder() { + val orderedSpaces = listOf( + "roomId1" to "aaaa", + "roomId2" to "ffff", + "roomId3" to "pppp", + "roomId4" to null, + "roomId5" to null, + "roomId6" to null + ).assertSpaceOrdered() + + val orderCommand = SpaceOrderUtils.orderCommandsForMove(orderedSpaces, "roomId5", -4) + + Assert.assertEquals("1 order change should be needed", 1, orderCommand.size) + + val reOrdered = reOrderWithCommands(orderedSpaces, orderCommand) + + Assert.assertEquals("roomId5", reOrdered[0].first) + Assert.assertEquals("roomId1", reOrdered[1].first) + Assert.assertEquals("roomId2", reOrdered[2].first) + Assert.assertEquals("roomId3", reOrdered[3].first) + Assert.assertEquals("roomId4", reOrdered[4].first) + Assert.assertEquals("roomId6", reOrdered[5].first) + } + + @Test + fun testMultipleMoveOrder() { + val orderedSpaces = listOf( + "roomId1" to null, + "roomId2" to null, + "roomId3" to null, + "roomId4" to null, + "roomId5" to null, + "roomId6" to null + ).assertSpaceOrdered() + + // move 5 to Top + val fiveToTop = SpaceOrderUtils.orderCommandsForMove(orderedSpaces, "roomId5", -4) + + val fiveTopReOrder = reOrderWithCommands(orderedSpaces, fiveToTop) + + // now move 4 to second + val orderCommand = SpaceOrderUtils.orderCommandsForMove(fiveTopReOrder, "roomId4", -3) + + val reOrdered = reOrderWithCommands(fiveTopReOrder, orderCommand) + // second order should cost 1 re-order + Assert.assertEquals(1, orderCommand.size) + + Assert.assertEquals("roomId5", reOrdered[0].first) + Assert.assertEquals("roomId4", reOrdered[1].first) + Assert.assertEquals("roomId1", reOrdered[2].first) + Assert.assertEquals("roomId2", reOrdered[3].first) + Assert.assertEquals("roomId3", reOrdered[4].first) + Assert.assertEquals("roomId6", reOrdered[5].first) + } + + private fun reOrderWithCommands(orderedSpaces: List>, orderCommand: List) = + orderedSpaces.map { orderInfo -> + orderInfo.first to (orderCommand.find { it.spaceId == orderInfo.first }?.order ?: orderInfo.second) + } + .sortedWith(TestSpaceComparator()) + + private fun List>.assertSpaceOrdered() : List> { + assertEquals(this, this.sortedWith(TestSpaceComparator())) + return this + } + + class TestSpaceComparator : Comparator> { + + override fun compare(left: Pair?, right: Pair?): Int { + val leftOrder = left?.second + val rightOrder = right?.second + return if (leftOrder != null && rightOrder != null) { + leftOrder.compareTo(rightOrder) + } else { + if (leftOrder == null) { + if (rightOrder == null) { + compareValues(left?.first, right?.first) + } else { + 1 + } + } else { + -1 + } + } + } + } +} diff --git a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/StringOrderTest.kt b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/StringOrderTest.kt new file mode 100644 index 00000000000..a625362c041 --- /dev/null +++ b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/StringOrderTest.kt @@ -0,0 +1,110 @@ +/* + * Copyright 2021 The Matrix.org Foundation C.I.C. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.matrix.android.sdk + +import org.amshove.kluent.internal.assertEquals +import org.junit.Assert +import org.junit.Test +import org.matrix.android.sdk.api.MatrixPatterns +import org.matrix.android.sdk.api.util.StringOrderUtils + +class StringOrderTest { + + @Test + fun testbasing() { + assertEquals("a", StringOrderUtils.baseToString(StringOrderUtils.stringToBase("a", StringOrderUtils.DEFAULT_ALPHABET), StringOrderUtils.DEFAULT_ALPHABET)) + assertEquals("element", StringOrderUtils.baseToString(StringOrderUtils.stringToBase("element", StringOrderUtils.DEFAULT_ALPHABET), StringOrderUtils.DEFAULT_ALPHABET)) + assertEquals("matrix", StringOrderUtils.baseToString(StringOrderUtils.stringToBase("matrix", StringOrderUtils.DEFAULT_ALPHABET), StringOrderUtils.DEFAULT_ALPHABET)) + } + + @Test + fun testValid() { + println(StringOrderUtils.DEFAULT_ALPHABET.joinToString(",")) + + assert(MatrixPatterns.isValidOrderString("a")) + assert(MatrixPatterns.isValidOrderString(" ")) + assert(MatrixPatterns.isValidOrderString("abc")) + assert(!MatrixPatterns.isValidOrderString("abcê")) + assert(!MatrixPatterns.isValidOrderString("")) + assert(MatrixPatterns.isValidOrderString("!")) + assert(MatrixPatterns.isValidOrderString("!\"#\$%&'()*+,012")) + assert(!MatrixPatterns.isValidOrderString(Char(' '.code - 1).toString())) + + assert(!MatrixPatterns.isValidOrderString( + buildString { + for (i in 0..49) { + append(StringOrderUtils.DEFAULT_ALPHABET.random()) + } + } + )) + + assert(MatrixPatterns.isValidOrderString( + buildString { + for (i in 0..48) { + append(StringOrderUtils.DEFAULT_ALPHABET.random()) + } + } + )) + } + + @Test + fun testAverage() { + assertAverage("${StringOrderUtils.DEFAULT_ALPHABET.first()}", "m") + assertAverage("aa", "aab") + assertAverage("matrix", "element") + assertAverage("mmm", "mmmmm") + assertAverage("aab", "aa") + assertAverage("", "aa") + assertAverage("a", "z") + assertAverage("ground", "sky") + } + + @Test + fun testMidPoints() { + val orders = StringOrderUtils.midPoints("element", "matrix", 4) + assertEquals(4, orders!!.size) + assert("element" < orders[0]) + assert(orders[0] < orders[1]) + assert(orders[1] < orders[2]) + assert(orders[2] < orders[3]) + assert(orders[3] < "matrix") + + println("element < ${orders.joinToString(" < ") { "[$it]" }} < matrix") + + val orders2 = StringOrderUtils.midPoints("a", "d", 4) + assertEquals(null, orders2) + } + + @Test + fun testRenumberNeeded() { + assertEquals(null, StringOrderUtils.average("a", "a")) + assertEquals(null, StringOrderUtils.average("", "")) + assertEquals(null, StringOrderUtils.average("a", "b")) + assertEquals(null, StringOrderUtils.average("b", "a")) + assertEquals(null, StringOrderUtils.average("mmmm", "mmmm")) + assertEquals(null, StringOrderUtils.average("a${Char(0)}", "a")) + } + + private fun assertAverage(first: String, second: String) { + val left = first.takeIf { first < second } ?: second + val right = first.takeIf { first > second } ?: second + val av1 = StringOrderUtils.average(left, right)!! + println("[$left] < [$av1] < [$right]") + Assert.assertTrue(left < av1) + Assert.assertTrue(av1 < right) + } +} diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/MatrixPatterns.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/MatrixPatterns.kt index d8532c77c83..841e8332711 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/MatrixPatterns.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/MatrixPatterns.kt @@ -71,6 +71,9 @@ object MatrixPatterns { private const val LINK_TO_APP_ROOM_ALIAS_REGEXP = APP_BASE_REGEX + MATRIX_ROOM_ALIAS_REGEX + SEP_REGEX + MATRIX_EVENT_IDENTIFIER_REGEX private val PATTERN_CONTAIN_APP_LINK_PERMALINK_ROOM_ALIAS = LINK_TO_APP_ROOM_ALIAS_REGEXP.toRegex(RegexOption.IGNORE_CASE) + // ascii characters in the range \x20 (space) to \x7E (~) + val ORDER_STRING_REGEX = "[ -~]+".toRegex() + // list of patterns to find some matrix item. val MATRIX_PATTERNS = listOf( PATTERN_CONTAIN_MATRIX_TO_PERMALINK_ROOM_ID, @@ -146,4 +149,12 @@ object MatrixPatterns { fun extractServerNameFromId(matrixId: String?): String? { return matrixId?.substringAfter(":", missingDelimiterValue = "")?.takeIf { it.isNotEmpty() } } + + /** + * Orders which are not strings, or do not consist solely of ascii characters in the range \x20 (space) to \x7E (~), + * or consist of more than 50 characters, are forbidden and the field should be ignored if received. + */ + fun isValidOrderString(order: String?) : Boolean { + return order != null && order.length < 50 && order matches ORDER_STRING_REGEX + } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/accountdata/RoomAccountDataTypes.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/accountdata/RoomAccountDataTypes.kt index 0e80c307b4f..96eb86c0d65 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/accountdata/RoomAccountDataTypes.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/accountdata/RoomAccountDataTypes.kt @@ -20,4 +20,5 @@ object RoomAccountDataTypes { const val EVENT_TYPE_VIRTUAL_ROOM = "im.vector.is_virtual_room" const val EVENT_TYPE_TAG = "m.tag" const val EVENT_TYPE_FULLY_READ = "m.fully_read" + const val EVENT_TYPE_SPACE_ORDER = "org.matrix.msc3230.space_order" // m.space_order } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/space/SpaceOrderUtils.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/space/SpaceOrderUtils.kt new file mode 100644 index 00000000000..3385662a5e8 --- /dev/null +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/space/SpaceOrderUtils.kt @@ -0,0 +1,92 @@ +/* + * Copyright 2021 The Matrix.org Foundation C.I.C. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.matrix.android.sdk.api.session.space + +import org.matrix.android.sdk.api.util.StringOrderUtils + +object SpaceOrderUtils { + + data class SpaceReOrderCommand( + val spaceId: String, + val order: String + ) + + fun orderCommandsForMove(orderedSpacesToOrderMap: List>, movedSpaceId: String, delta: Int): List { + val movedIndex = orderedSpacesToOrderMap.indexOfFirst { it.first == movedSpaceId } + if (movedIndex == -1) return emptyList() + if (delta == 0) return emptyList() + + val targetIndex = if (delta > 0) movedIndex + delta else (movedIndex + delta - 1) + + val nodesToReNumber = mutableListOf() + var lowerBondOrder: String? = null + var afterSpace: Pair? = null +// if (delta > 0) { + var index = targetIndex + while (index >= 0 && lowerBondOrder == null) { + val node = orderedSpacesToOrderMap.getOrNull(index) + if (node != null /*null when adding at the end*/) { + val nodeOrder = node.second + if (node.first == movedSpaceId) break + if (nodeOrder == null) { + nodesToReNumber.add(0, node.first) + } else { + lowerBondOrder = nodeOrder + } + } + index-- + } + nodesToReNumber.add(movedSpaceId) + afterSpace = if (orderedSpacesToOrderMap.indices.contains(targetIndex + 1)) orderedSpacesToOrderMap[targetIndex + 1] else null + + val defaultMaxOrder = CharArray(4) { StringOrderUtils.DEFAULT_ALPHABET.last() } + .joinToString("") + + val defaultMinOrder = CharArray(4) { StringOrderUtils.DEFAULT_ALPHABET.first() } + .joinToString("") + + val afterOrder = afterSpace?.second ?: defaultMaxOrder + + val beforeOrder = lowerBondOrder ?: defaultMinOrder + + val newOrder = StringOrderUtils.midPoints(beforeOrder, afterOrder, nodesToReNumber.size) + + if (newOrder.isNullOrEmpty()) { + // re order all? + val expectedList = orderedSpacesToOrderMap.toMutableList() + expectedList.removeAt(movedIndex).let { + expectedList.add(movedIndex + delta, it) + } + + return StringOrderUtils.midPoints(defaultMinOrder, defaultMaxOrder, orderedSpacesToOrderMap.size)?.let { orders -> + expectedList.mapIndexed { index, pair -> + SpaceReOrderCommand( + pair.first, + orders[index] + ) + } + } ?: emptyList() + } else { + return nodesToReNumber.mapIndexed { index, s -> + SpaceReOrderCommand( + s, + newOrder[index] + ) + } + } + } +} diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/space/model/SpaceOrderContent.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/space/model/SpaceOrderContent.kt new file mode 100644 index 00000000000..a8578347c87 --- /dev/null +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/space/model/SpaceOrderContent.kt @@ -0,0 +1,37 @@ +/* + * Copyright 2021 The Matrix.org Foundation C.I.C. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.matrix.android.sdk.api.session.space.model + +import com.squareup.moshi.JsonClass +import org.matrix.android.sdk.api.MatrixPatterns + +/** + * { + * "type": "m.space_order", + * "content": { + * "order": "..." + * } + * } + */ +@JsonClass(generateAdapter = true) +data class SpaceOrderContent( + val order: String? = null +) { + fun safeOrder(): String? { + return order?.takeIf { MatrixPatterns.isValidOrderString(it) } + } +} diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/space/model/TopLevelSpaceComparator.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/space/model/TopLevelSpaceComparator.kt new file mode 100644 index 00000000000..e7289a277c5 --- /dev/null +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/space/model/TopLevelSpaceComparator.kt @@ -0,0 +1,44 @@ +/* + * Copyright (c) 2021 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.matrix.android.sdk.api.session.space.model + +import org.matrix.android.sdk.api.session.room.model.RoomSummary + +// Can't use regular compare by because Null is considered less than any value, and for space order it's the opposite +class TopLevelSpaceComparator(val orders: Map) : Comparator { + + override fun compare(left: RoomSummary?, right: RoomSummary?): Int { + val leftOrder = left?.roomId?.let { orders[it] } + val rightOrder = right?.roomId?.let { orders[it] } + return if (leftOrder != null && rightOrder != null) { + leftOrder.compareTo(rightOrder) + } else { + if (leftOrder == null) { + if (rightOrder == null) { + compareValues(left?.roomId, right?.roomId) + } else { + 1 + } + } else { + -1 + } + } +// .also { +// Timber.w("VAL: compare(${left?.displayName} | $leftOrder ,${right?.displayName} | $rightOrder) = $it") +// } + } +} diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/util/StringOrderUtils.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/util/StringOrderUtils.kt new file mode 100644 index 00000000000..83c85859416 --- /dev/null +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/util/StringOrderUtils.kt @@ -0,0 +1,87 @@ +/* + * Copyright 2021 The Matrix.org Foundation C.I.C. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.matrix.android.sdk.api.util + +import java.math.BigInteger + +object StringOrderUtils { + + val DEFAULT_ALPHABET = buildString { + for (i in 0x20..0x7E) { + append(Char(i)) + } + }.toCharArray() + + // /=Range(0x20, 0x7E) + + fun average(left: String, right: String, alphabet: CharArray = DEFAULT_ALPHABET): String? { + return midPoints(left, right, 1, alphabet)?.firstOrNull() + } + + fun midPoints(left: String, right: String, count: Int, alphabet: CharArray = DEFAULT_ALPHABET): List? { + if (left == right) return null // no space in between.. + if (left > right) return midPoints(right, left, count, alphabet) + val size = maxOf(left.length, right.length) + val leftPadded = pad(left, size, alphabet.first()) + val rightPadded = pad(right, size, alphabet.first()) + val b1 = stringToBase(leftPadded, alphabet) + val b2 = stringToBase(rightPadded, alphabet) + val step = (b2.minus(b1)).div(BigInteger("${count + 1}")) + val orders = mutableListOf() + var previous = left + for (i in 0 until count) { + val newOrder = baseToString(b1.add(step.multiply(BigInteger("${i + 1}"))), alphabet) + orders.add(newOrder) + // ensure there was enought precision + if (previous >= newOrder) return null + previous = newOrder + } + return orders.takeIf { orders.last() < right } + } + + private fun pad(string: String, size: Int, padding: Char): String { + val raw = string.toCharArray() + return CharArray(size).also { + for (index in it.indices) { + if (index < raw.size) { + it[index] = raw[index] + } else { + it[index] = padding + } + } + }.joinToString("") + } + + fun baseToString(x: BigInteger, alphabet: CharArray): String { + val len = alphabet.size.toBigInteger() + if (x < len) { + return alphabet[x.toInt()].toString() + } else { + return baseToString(x.div(len), alphabet) + alphabet[x.rem(len).toInt()].toString() + } + } + + fun stringToBase(x: String, alphabet: CharArray): BigInteger { + if (x.isEmpty()) throw IllegalArgumentException() + val len = alphabet.size.toBigInteger() + var result = BigInteger("0") + x.reversed().forEachIndexed { index, c -> + result += (alphabet.indexOf(c).toBigInteger() * len.pow(index)) + } + return result + } +} diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/accountdata/DefaultRoomAccountDataService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/accountdata/DefaultRoomAccountDataService.kt index d43c1d32176..caeeb3bf537 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/accountdata/DefaultRoomAccountDataService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/accountdata/DefaultRoomAccountDataService.kt @@ -26,8 +26,8 @@ import org.matrix.android.sdk.api.session.room.accountdata.RoomAccountDataServic import org.matrix.android.sdk.api.util.Optional internal class DefaultRoomAccountDataService @AssistedInject constructor(@Assisted private val roomId: String, - private val dataSource: RoomAccountDataDataSource, - private val updateRoomAccountDataTask: UpdateRoomAccountDataTask + private val dataSource: RoomAccountDataDataSource, + private val updateRoomAccountDataTask: UpdateRoomAccountDataTask ) : RoomAccountDataService { @AssistedFactory diff --git a/vector/build.gradle b/vector/build.gradle index 21f62647c00..67aed7cf152 100644 --- a/vector/build.gradle +++ b/vector/build.gradle @@ -366,6 +366,7 @@ dependencies { implementation "com.jakewharton.rxbinding3:rxbinding-appcompat:$rxbinding_version" implementation "com.jakewharton.rxbinding3:rxbinding-material:$rxbinding_version" +// implementation fileTree(include: ['*.jar', '*.aar'], dir: 'libs') implementation("com.airbnb.android:epoxy:$epoxy_version") implementation "com.airbnb.android:epoxy-glide-preloading:$epoxy_version" kapt "com.airbnb.android:epoxy-processor:$epoxy_version" diff --git a/vector/src/main/java/im/vector/app/features/spaces/SpaceListAction.kt b/vector/src/main/java/im/vector/app/features/spaces/SpaceListAction.kt index b8b6cb46670..12d4b40f427 100644 --- a/vector/src/main/java/im/vector/app/features/spaces/SpaceListAction.kt +++ b/vector/src/main/java/im/vector/app/features/spaces/SpaceListAction.kt @@ -26,6 +26,9 @@ sealed class SpaceListAction : VectorViewModelAction { data class LeaveSpace(val spaceSummary: RoomSummary) : SpaceListAction() data class ToggleExpand(val spaceSummary: RoomSummary) : SpaceListAction() object AddSpace : SpaceListAction() + data class MoveSpace(val spaceId: String, val delta : Int) : SpaceListAction() + data class OnStartDragging(val spaceId: String, val expanded: Boolean) : SpaceListAction() + data class OnEndDragging(val spaceId: String, val expanded: Boolean) : SpaceListAction() data class SelectLegacyGroup(val groupSummary: GroupSummary?) : SpaceListAction() } diff --git a/vector/src/main/java/im/vector/app/features/spaces/SpaceListFragment.kt b/vector/src/main/java/im/vector/app/features/spaces/SpaceListFragment.kt index f50ba902212..0692ddd3fc1 100644 --- a/vector/src/main/java/im/vector/app/features/spaces/SpaceListFragment.kt +++ b/vector/src/main/java/im/vector/app/features/spaces/SpaceListFragment.kt @@ -20,6 +20,7 @@ import android.os.Bundle import android.view.LayoutInflater import android.view.View import android.view.ViewGroup +import com.airbnb.epoxy.EpoxyTouchHelper import com.airbnb.mvrx.Incomplete import com.airbnb.mvrx.Success import com.airbnb.mvrx.fragmentViewModel @@ -54,6 +55,43 @@ class SpaceListFragment @Inject constructor( spaceController.callback = this views.stateView.contentView = views.groupListView views.groupListView.configureWith(spaceController) + EpoxyTouchHelper.initDragging(spaceController) + .withRecyclerView(views.groupListView) + .forVerticalList() + .withTarget(SpaceSummaryItem::class.java) + .andCallbacks(object : EpoxyTouchHelper.DragCallbacks() { + var toPositionM: Int? = null + var fromPositionM: Int? = null + + override fun onDragStarted(model: SpaceSummaryItem?, itemView: View?, adapterPosition: Int) { + super.onDragStarted(model, itemView, adapterPosition) + toPositionM = null + fromPositionM = null + model?.matrixItem?.id?.let { + viewModel.handle(SpaceListAction.OnStartDragging(it, model.expanded)) + } + } + + override fun onDragReleased(model: SpaceSummaryItem?, itemView: View?) { +// Timber.v("VAL: onModelMoved from $fromPositionM to $toPositionM ${model?.matrixItem?.getBestName()}") + if (toPositionM == null || fromPositionM == null) return + val movingSpace = model?.matrixItem?.id ?: return + viewModel.handle(SpaceListAction.MoveSpace(movingSpace, toPositionM!! - fromPositionM!!)) + } + override fun onModelMoved(fromPosition: Int, toPosition: Int, modelBeingMoved: SpaceSummaryItem?, itemView: View?) { +// Timber.v("VAL: onModelMoved incremental from $fromPosition to $toPosition ${modelBeingMoved?.matrixItem?.getBestName()}") + if (fromPositionM == null) { + fromPositionM = fromPosition + } + toPositionM = toPosition + } + + override fun isDragEnabledForModel(model: SpaceSummaryItem?): Boolean { +// Timber.v("VAL: isDragEnabledForModel ${model?.matrixItem?.getBestName()}") + return model?.canDrag == true + } + }) + viewModel.observeViewEvents { when (it) { is SpaceListViewEvents.OpenSpaceSummary -> sharedActionViewModel.post(HomeActivitySharedAction.OpenSpacePreview(it.id)) diff --git a/vector/src/main/java/im/vector/app/features/spaces/SpaceListViewState.kt b/vector/src/main/java/im/vector/app/features/spaces/SpaceListViewState.kt index b57a5d364b0..7482f4881e9 100644 --- a/vector/src/main/java/im/vector/app/features/spaces/SpaceListViewState.kt +++ b/vector/src/main/java/im/vector/app/features/spaces/SpaceListViewState.kt @@ -29,7 +29,9 @@ data class SpaceListViewState( val myMxItem : Async = Uninitialized, val asyncSpaces: Async> = Uninitialized, val selectedGroupingMethod: RoomGroupingMethod = RoomGroupingMethod.BySpace(null), - val rootSpaces: List? = null, + val rootSpacesOrdered: List? = null, + val spaceOrderInfo: Map? = null, + val spaceOrderLocalEchos: Map? = null, val legacyGroups: List? = null, val expandedStates: Map = emptyMap(), val homeAggregateCount : RoomAggregateNotificationCount = RoomAggregateNotificationCount(0, 0) diff --git a/vector/src/main/java/im/vector/app/features/spaces/SpaceSummaryController.kt b/vector/src/main/java/im/vector/app/features/spaces/SpaceSummaryController.kt index 78eb232cf65..fd58af0408f 100644 --- a/vector/src/main/java/im/vector/app/features/spaces/SpaceSummaryController.kt +++ b/vector/src/main/java/im/vector/app/features/spaces/SpaceSummaryController.kt @@ -62,9 +62,10 @@ class SpaceSummaryController @Inject constructor( buildGroupModels( nonNullViewState.asyncSpaces(), nonNullViewState.selectedGroupingMethod, - nonNullViewState.rootSpaces, + nonNullViewState.rootSpacesOrdered, nonNullViewState.expandedStates, - nonNullViewState.homeAggregateCount) + nonNullViewState.homeAggregateCount, + nonNullViewState.spaceOrderInfo) if (!nonNullViewState.legacyGroups.isNullOrEmpty()) { genericFooterItem { @@ -107,7 +108,8 @@ class SpaceSummaryController @Inject constructor( selected: RoomGroupingMethod, rootSpaces: List?, expandedStates: Map, - homeCount: RoomAggregateNotificationCount) { + homeCount: RoomAggregateNotificationCount, + spaceOrderInfo: Map?) { val host = this spaceBetaHeaderItem { id("beta_header") @@ -127,6 +129,7 @@ class SpaceSummaryController @Inject constructor( countState(UnreadCounterBadgeView.State(1, true)) selected(false) description(host.stringProvider.getString(R.string.you_are_invited)) + canDrag(false) listener { host.callback?.onSpaceInviteSelected(roomSummary) } } } @@ -139,7 +142,6 @@ class SpaceSummaryController @Inject constructor( } rootSpaces - ?.sortedBy { it.roomId } ?.forEach { groupSummary -> val isSelected = selected is RoomGroupingMethod.BySpace && groupSummary.roomId == selected.space()?.roomId // does it have children? @@ -154,8 +156,12 @@ class SpaceSummaryController @Inject constructor( id(groupSummary.roomId) hasChildren(hasChildren) expanded(expanded) + // to debug order + // matrixItem(groupSummary.copy(displayName = "${groupSummary.displayName} / ${spaceOrderInfo?.get(groupSummary.roomId)}") + // .toMatrixItem()) matrixItem(groupSummary.toMatrixItem()) selected(isSelected) + canDrag(true) onMore { host.callback?.onSpaceSettings(groupSummary) } listener { host.callback?.onSpaceSelected(groupSummary) } toggleExpand { host.callback?.onToggleExpand(groupSummary) } diff --git a/vector/src/main/java/im/vector/app/features/spaces/SpaceSummaryItem.kt b/vector/src/main/java/im/vector/app/features/spaces/SpaceSummaryItem.kt index a7432571ede..6cffabd8513 100644 --- a/vector/src/main/java/im/vector/app/features/spaces/SpaceSummaryItem.kt +++ b/vector/src/main/java/im/vector/app/features/spaces/SpaceSummaryItem.kt @@ -51,6 +51,7 @@ abstract class SpaceSummaryItem : VectorEpoxyModel() { @EpoxyAttribute var countState: UnreadCounterBadgeView.State = UnreadCounterBadgeView.State(0, false) @EpoxyAttribute var description: String? = null @EpoxyAttribute var showSeparator: Boolean = false + @EpoxyAttribute var canDrag: Boolean = true override fun bind(holder: Holder) { super.bind(holder) diff --git a/vector/src/main/java/im/vector/app/features/spaces/SpacesListViewModel.kt b/vector/src/main/java/im/vector/app/features/spaces/SpacesListViewModel.kt index d7d7af4ca5e..73fcf0a2495 100644 --- a/vector/src/main/java/im/vector/app/features/spaces/SpacesListViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/spaces/SpacesListViewModel.kt @@ -28,23 +28,30 @@ import dagger.assisted.AssistedInject import im.vector.app.AppStateHandler import im.vector.app.RoomGroupingMethod import im.vector.app.core.platform.VectorViewModel +import im.vector.app.features.session.coroutineScope import im.vector.app.features.settings.VectorPreferences import im.vector.app.group import im.vector.app.space import io.reactivex.Observable -import io.reactivex.functions.BiFunction import io.reactivex.schedulers.Schedulers import kotlinx.coroutines.launch import org.matrix.android.sdk.api.extensions.tryOrNull import org.matrix.android.sdk.api.query.ActiveSpaceFilter import org.matrix.android.sdk.api.query.QueryStringValue import org.matrix.android.sdk.api.session.Session +import org.matrix.android.sdk.api.session.events.model.toContent +import org.matrix.android.sdk.api.session.events.model.toModel import org.matrix.android.sdk.api.session.group.groupSummaryQueryParams import org.matrix.android.sdk.api.session.room.RoomSortOrder +import org.matrix.android.sdk.api.session.room.accountdata.RoomAccountDataEvent +import org.matrix.android.sdk.api.session.room.accountdata.RoomAccountDataTypes import org.matrix.android.sdk.api.session.room.model.Membership import org.matrix.android.sdk.api.session.room.model.RoomSummary import org.matrix.android.sdk.api.session.room.roomSummaryQueryParams import org.matrix.android.sdk.api.session.room.summary.RoomAggregateNotificationCount +import org.matrix.android.sdk.api.session.space.SpaceOrderUtils +import org.matrix.android.sdk.api.session.space.model.SpaceOrderContent +import org.matrix.android.sdk.api.session.space.model.TopLevelSpaceComparator import org.matrix.android.sdk.api.session.user.model.User import org.matrix.android.sdk.api.util.toMatrixItem import org.matrix.android.sdk.rx.asObservable @@ -143,37 +150,86 @@ class SpacesListViewModel @AssistedInject constructor(@Assisted initialState: Sp }.disposeOnClear() } -// private fun observeSelectionState() { -// selectSubscribe(SpaceListViewState::selectedSpace) { spaceSummary -> -// if (spaceSummary != null) { -// // We only want to open group if the updated selectedGroup is a different one. -// if (currentGroupId != spaceSummary.roomId) { -// currentGroupId = spaceSummary.roomId -// _viewEvents.post(SpaceListViewEvents.OpenSpace) -// } -// appStateHandler.setCurrentSpace(spaceSummary.roomId) -// } else { -// // If selected group is null we force to default. It can happens when leaving the selected group. -// setState { -// copy(selectedSpace = this.asyncSpaces()?.find { it.roomId == ALL_COMMUNITIES_GROUP_ID }) -// } -// } -// } -// } - override fun handle(action: SpaceListAction) { when (action) { - is SpaceListAction.SelectSpace -> handleSelectSpace(action) - is SpaceListAction.LeaveSpace -> handleLeaveSpace(action) - SpaceListAction.AddSpace -> handleAddSpace() - is SpaceListAction.ToggleExpand -> handleToggleExpand(action) - is SpaceListAction.OpenSpaceInvite -> handleSelectSpaceInvite(action) + is SpaceListAction.SelectSpace -> handleSelectSpace(action) + is SpaceListAction.LeaveSpace -> handleLeaveSpace(action) + SpaceListAction.AddSpace -> handleAddSpace() + is SpaceListAction.ToggleExpand -> handleToggleExpand(action) + is SpaceListAction.OpenSpaceInvite -> handleSelectSpaceInvite(action) is SpaceListAction.SelectLegacyGroup -> handleSelectGroup(action) + is SpaceListAction.MoveSpace -> handleMoveSpace(action) + is SpaceListAction.OnEndDragging -> handleEndDragging(action) + is SpaceListAction.OnStartDragging -> handleStartDragging(action) } } // PRIVATE METHODS ***************************************************************************** + var preDragExpandedState: Map? = null + private fun handleStartDragging(action: SpaceListAction.OnStartDragging) = withState { state -> + preDragExpandedState = state.expandedStates.toMap() + setState { + copy( + expandedStates = expandedStates.map { + it.key to false + }.toMap() + ) + } + } + + private fun handleEndDragging(action: SpaceListAction.OnEndDragging) = withState { state -> + // restore expanded state + setState { + copy( + expandedStates = preDragExpandedState.orEmpty() + ) + } + } + + private fun handleMoveSpace(action: SpaceListAction.MoveSpace) = withState { state -> + state.rootSpacesOrdered ?: return@withState + val orderCommands = SpaceOrderUtils.orderCommandsForMove( + state.rootSpacesOrdered.map { + it.roomId to (state.spaceOrderLocalEchos?.get(it.roomId) ?: state.spaceOrderInfo?.get(it.roomId)) + }, + action.spaceId, + action.delta + ) + + // local echo + val updatedLocalEchos = state.spaceOrderLocalEchos.orEmpty().toMutableMap().apply { + orderCommands.forEach { + this[it.spaceId] = it.order + } + }.toMap() + + setState { + copy( + rootSpacesOrdered = state.rootSpacesOrdered.toMutableList().apply { + val index = indexOfFirst { it.roomId == action.spaceId } + val moved = removeAt(index) + add(index + action.delta, moved) + }, + spaceOrderLocalEchos = updatedLocalEchos + ) + } + session.coroutineScope.launch { + orderCommands.forEach { + session.getRoom(it.spaceId)?.updateAccountData(RoomAccountDataTypes.EVENT_TYPE_SPACE_ORDER, + SpaceOrderContent(order = it.order).toContent() + ) + } + } + + // restore expanded state + setState { + copy( + expandedStates = preDragExpandedState.orEmpty() + ) + } + } + private fun handleSelectSpace(action: SpaceListAction.SelectSpace) = withState { state -> val groupingMethod = state.selectedGroupingMethod if (groupingMethod is RoomGroupingMethod.ByLegacyGroup || groupingMethod.space()?.roomId != action.spaceSummary?.roomId) { @@ -224,24 +280,43 @@ class SpacesListViewModel @AssistedInject constructor(@Assisted initialState: Sp excludeType = listOf(/**RoomType.MESSAGING,$*/ null) } - Observable.combineLatest, List>( - session - .rx() + + val rxSession = session.rx() + + Observable.combineLatest, List, List>( + rxSession .liveUser(session.myUserId) .map { it.getOrNull() }, - session - .rx() + rxSession .liveSpaceSummaries(spaceSummaryQueryParams), - BiFunction { _, communityGroups -> + session.accountDataService().getLiveRoomAccountDataEvents(setOf(RoomAccountDataTypes.EVENT_TYPE_SPACE_ORDER)).asObservable(), + { _, communityGroups, _ -> communityGroups } ) .execute { async -> + val rootSpaces = session.spaceService().getRootSpaceSummaries() + val orders = rootSpaces.map { + it.roomId to session.getRoom(it.roomId) + ?.getAccountDataEvent(RoomAccountDataTypes.EVENT_TYPE_SPACE_ORDER) + ?.content.toModel() + ?.safeOrder() + }.toMap() copy( asyncSpaces = async, - rootSpaces = session.spaceService().getRootSpaceSummaries() + rootSpacesOrdered = rootSpaces.sortedWith(TopLevelSpaceComparator(orders)), + spaceOrderInfo = orders + ) + } + + // clear local echos on update + session.accountDataService() + .getLiveRoomAccountDataEvents(setOf(RoomAccountDataTypes.EVENT_TYPE_SPACE_ORDER)) + .asObservable().execute { + copy( + spaceOrderLocalEchos = emptyMap() ) } } From 1de74b1c921bd7da07edcdd61df34c62b6a84ee7 Mon Sep 17 00:00:00 2001 From: Valere Date: Mon, 14 Jun 2021 15:20:14 +0200 Subject: [PATCH 2/5] Update change log --- build.gradle | 3 --- newsfragment/3501.feature | 1 + 2 files changed, 1 insertion(+), 3 deletions(-) create mode 100644 newsfragment/3501.feature diff --git a/build.gradle b/build.gradle index 92dedbe1af4..b2130664233 100644 --- a/build.gradle +++ b/build.gradle @@ -10,9 +10,6 @@ buildscript { maven { url "https://plugins.gradle.org/m2/" } -// flatDir { -// dirs 'libs' -// } } dependencies { classpath 'com.android.tools.build:gradle:4.2.1' diff --git a/newsfragment/3501.feature b/newsfragment/3501.feature new file mode 100644 index 00000000000..c0602dda3f4 --- /dev/null +++ b/newsfragment/3501.feature @@ -0,0 +1 @@ +User defined top level spaces ordering (#3501) \ No newline at end of file From 944c9641a9ebe358a37fb0c6498c704b982c8a4b Mon Sep 17 00:00:00 2001 From: Valere Date: Thu, 17 Jun 2021 14:44:02 +0200 Subject: [PATCH 3/5] Code review --- .../org/matrix/android/sdk/SpaceOrderTest.kt | 37 +++++++------------ .../sdk/api/session/space/SpaceOrderUtils.kt | 23 +++++++++--- vector/build.gradle | 1 - 3 files changed, 32 insertions(+), 29 deletions(-) diff --git a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/SpaceOrderTest.kt b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/SpaceOrderTest.kt index e8fdf5472ae..b9a92585c61 100644 --- a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/SpaceOrderTest.kt +++ b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/SpaceOrderTest.kt @@ -235,35 +235,26 @@ class SpaceOrderTest { Assert.assertEquals("roomId6", reOrdered[5].first) } + @Test + fun testComparator() { + listOf( + "roomId2" to "a", + "roomId1" to "b", + "roomId3" to null, + "roomId4" to null, + ).assertSpaceOrdered() + } + private fun reOrderWithCommands(orderedSpaces: List>, orderCommand: List) = orderedSpaces.map { orderInfo -> orderInfo.first to (orderCommand.find { it.spaceId == orderInfo.first }?.order ?: orderInfo.second) } - .sortedWith(TestSpaceComparator()) + .sortedWith(testSpaceComparator) - private fun List>.assertSpaceOrdered() : List> { - assertEquals(this, this.sortedWith(TestSpaceComparator())) + private fun List>.assertSpaceOrdered(): List> { + assertEquals(this, this.sortedWith(testSpaceComparator)) return this } - class TestSpaceComparator : Comparator> { - - override fun compare(left: Pair?, right: Pair?): Int { - val leftOrder = left?.second - val rightOrder = right?.second - return if (leftOrder != null && rightOrder != null) { - leftOrder.compareTo(rightOrder) - } else { - if (leftOrder == null) { - if (rightOrder == null) { - compareValues(left?.first, right?.first) - } else { - 1 - } - } else { - -1 - } - } - } - } + private val testSpaceComparator = compareBy, String?>(nullsLast()) { it.second }.thenBy { it.first } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/space/SpaceOrderUtils.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/space/SpaceOrderUtils.kt index 3385662a5e8..844a5adcb4a 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/space/SpaceOrderUtils.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/space/SpaceOrderUtils.kt @@ -18,6 +18,16 @@ package org.matrix.android.sdk.api.session.space import org.matrix.android.sdk.api.util.StringOrderUtils +/** + * Adds some utilities to compute correct string orders when ordering spaces. + * After moving a space (e.g via DnD), client should limit the number of room account data update. + * For example if the space is moved between two other spaces with orders, just update the moved space order by computing + * a mid point between the surrounding orders. + * If the space is moved after a space with no order, all the previous spaces should be then ordered, + * and the computed orders should be chosen so that there is enough gaps in between them to facilitate future re-order. + * Re numbering (i.e change all spaces m.space.order account data) should be avoided as much as possible, + * as the updates might not be atomic for other clients and would makes spaces jump around. + */ object SpaceOrderUtils { data class SpaceReOrderCommand( @@ -25,6 +35,9 @@ object SpaceOrderUtils { val order: String ) + /** + * Returns a minimal list of order change in order to re order the space list as per given move. + */ fun orderCommandsForMove(orderedSpacesToOrderMap: List>, movedSpaceId: String, delta: Int): List { val movedIndex = orderedSpacesToOrderMap.indexOfFirst { it.first == movedSpaceId } if (movedIndex == -1) return emptyList() @@ -34,8 +47,6 @@ object SpaceOrderUtils { val nodesToReNumber = mutableListOf() var lowerBondOrder: String? = null - var afterSpace: Pair? = null -// if (delta > 0) { var index = targetIndex while (index >= 0 && lowerBondOrder == null) { val node = orderedSpacesToOrderMap.getOrNull(index) @@ -51,7 +62,9 @@ object SpaceOrderUtils { index-- } nodesToReNumber.add(movedSpaceId) - afterSpace = if (orderedSpacesToOrderMap.indices.contains(targetIndex + 1)) orderedSpacesToOrderMap[targetIndex + 1] else null + val afterSpace: Pair? = if (orderedSpacesToOrderMap.indices.contains(targetIndex + 1)) { + orderedSpacesToOrderMap[targetIndex + 1] + } else null val defaultMaxOrder = CharArray(4) { StringOrderUtils.DEFAULT_ALPHABET.last() } .joinToString("") @@ -81,10 +94,10 @@ object SpaceOrderUtils { } } ?: emptyList() } else { - return nodesToReNumber.mapIndexed { index, s -> + return nodesToReNumber.mapIndexed { i, s -> SpaceReOrderCommand( s, - newOrder[index] + newOrder[i] ) } } diff --git a/vector/build.gradle b/vector/build.gradle index 67aed7cf152..21f62647c00 100644 --- a/vector/build.gradle +++ b/vector/build.gradle @@ -366,7 +366,6 @@ dependencies { implementation "com.jakewharton.rxbinding3:rxbinding-appcompat:$rxbinding_version" implementation "com.jakewharton.rxbinding3:rxbinding-material:$rxbinding_version" -// implementation fileTree(include: ['*.jar', '*.aar'], dir: 'libs') implementation("com.airbnb.android:epoxy:$epoxy_version") implementation "com.airbnb.android:epoxy-glide-preloading:$epoxy_version" kapt "com.airbnb.android:epoxy-processor:$epoxy_version" From 682e926965bae0b1e074a27fce2751d30c28e3ba Mon Sep 17 00:00:00 2001 From: Valere Date: Thu, 17 Jun 2021 15:53:35 +0200 Subject: [PATCH 4/5] Dnd enhancement + klint fix --- .../org/matrix/android/sdk/SpaceOrderTest.kt | 2 +- .../app/features/spaces/SpaceListFragment.kt | 18 +++++++++++++++--- vector/src/main/res/drawable/bg_space_item.xml | 2 +- 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/SpaceOrderTest.kt b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/SpaceOrderTest.kt index b9a92585c61..3270dfb7574 100644 --- a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/SpaceOrderTest.kt +++ b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/SpaceOrderTest.kt @@ -241,7 +241,7 @@ class SpaceOrderTest { "roomId2" to "a", "roomId1" to "b", "roomId3" to null, - "roomId4" to null, + "roomId4" to null ).assertSpaceOrdered() } diff --git a/vector/src/main/java/im/vector/app/features/spaces/SpaceListFragment.kt b/vector/src/main/java/im/vector/app/features/spaces/SpaceListFragment.kt index 0692ddd3fc1..0a67977e6cc 100644 --- a/vector/src/main/java/im/vector/app/features/spaces/SpaceListFragment.kt +++ b/vector/src/main/java/im/vector/app/features/spaces/SpaceListFragment.kt @@ -17,6 +17,7 @@ package im.vector.app.features.spaces import android.os.Bundle +import android.view.HapticFeedbackConstants import android.view.LayoutInflater import android.view.View import android.view.ViewGroup @@ -62,28 +63,38 @@ class SpaceListFragment @Inject constructor( .andCallbacks(object : EpoxyTouchHelper.DragCallbacks() { var toPositionM: Int? = null var fromPositionM: Int? = null + var initialElevation: Float? = null override fun onDragStarted(model: SpaceSummaryItem?, itemView: View?, adapterPosition: Int) { - super.onDragStarted(model, itemView, adapterPosition) toPositionM = null fromPositionM = null model?.matrixItem?.id?.let { viewModel.handle(SpaceListAction.OnStartDragging(it, model.expanded)) } + itemView?.performHapticFeedback(HapticFeedbackConstants.LONG_PRESS) + initialElevation = itemView?.elevation + itemView?.elevation = 6f } override fun onDragReleased(model: SpaceSummaryItem?, itemView: View?) { // Timber.v("VAL: onModelMoved from $fromPositionM to $toPositionM ${model?.matrixItem?.getBestName()}") if (toPositionM == null || fromPositionM == null) return val movingSpace = model?.matrixItem?.id ?: return - viewModel.handle(SpaceListAction.MoveSpace(movingSpace, toPositionM!! - fromPositionM!!)) + viewModel.handle(SpaceListAction.MoveSpace(movingSpace, toPositionM!! - fromPositionM!!)) } + + override fun clearView(model: SpaceSummaryItem?, itemView: View?) { +// Timber.v("VAL: clearView ${model?.matrixItem?.getBestName()}") + itemView?.elevation = initialElevation ?: 0f + } + override fun onModelMoved(fromPosition: Int, toPosition: Int, modelBeingMoved: SpaceSummaryItem?, itemView: View?) { // Timber.v("VAL: onModelMoved incremental from $fromPosition to $toPosition ${modelBeingMoved?.matrixItem?.getBestName()}") if (fromPositionM == null) { fromPositionM = fromPosition } toPositionM = toPosition + itemView?.performHapticFeedback(HapticFeedbackConstants.LONG_PRESS) } override fun isDragEnabledForModel(model: SpaceSummaryItem?): Boolean { @@ -112,7 +123,7 @@ class SpaceListFragment @Inject constructor( override fun invalidate() = withState(viewModel) { state -> when (state.asyncSpaces) { is Incomplete -> views.stateView.state = StateView.State.Loading - is Success -> views.stateView.state = StateView.State.Content + is Success -> views.stateView.state = StateView.State.Content } spaceController.update(state) } @@ -124,6 +135,7 @@ class SpaceListFragment @Inject constructor( override fun onSpaceInviteSelected(spaceSummary: RoomSummary) { viewModel.handle(SpaceListAction.OpenSpaceInvite(spaceSummary)) } + override fun onSpaceSettings(spaceSummary: RoomSummary) { sharedActionViewModel.post(HomeActivitySharedAction.ShowSpaceSettings(spaceSummary.roomId)) } diff --git a/vector/src/main/res/drawable/bg_space_item.xml b/vector/src/main/res/drawable/bg_space_item.xml index 0362a9a8df8..158a6769ba4 100644 --- a/vector/src/main/res/drawable/bg_space_item.xml +++ b/vector/src/main/res/drawable/bg_space_item.xml @@ -20,7 +20,7 @@ - + From 3dc7a6dc3411452f4a7a1531081e3e970574e1b3 Mon Sep 17 00:00:00 2001 From: Valere Date: Thu, 17 Jun 2021 17:00:28 +0200 Subject: [PATCH 5/5] cleaning, quality --- .../api/session/space/model/TopLevelSpaceComparator.kt | 2 +- .../vector/app/features/spaces/SpaceSummaryController.kt | 6 ++---- .../im/vector/app/features/spaces/SpacesListViewModel.kt | 8 ++++---- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/space/model/TopLevelSpaceComparator.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/space/model/TopLevelSpaceComparator.kt index e7289a277c5..8af4f3a149a 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/space/model/TopLevelSpaceComparator.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/space/model/TopLevelSpaceComparator.kt @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021 New Vector Ltd + * Copyright 2021 The Matrix.org Foundation C.I.C. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/vector/src/main/java/im/vector/app/features/spaces/SpaceSummaryController.kt b/vector/src/main/java/im/vector/app/features/spaces/SpaceSummaryController.kt index fd58af0408f..828308007bc 100644 --- a/vector/src/main/java/im/vector/app/features/spaces/SpaceSummaryController.kt +++ b/vector/src/main/java/im/vector/app/features/spaces/SpaceSummaryController.kt @@ -64,8 +64,7 @@ class SpaceSummaryController @Inject constructor( nonNullViewState.selectedGroupingMethod, nonNullViewState.rootSpacesOrdered, nonNullViewState.expandedStates, - nonNullViewState.homeAggregateCount, - nonNullViewState.spaceOrderInfo) + nonNullViewState.homeAggregateCount) if (!nonNullViewState.legacyGroups.isNullOrEmpty()) { genericFooterItem { @@ -108,8 +107,7 @@ class SpaceSummaryController @Inject constructor( selected: RoomGroupingMethod, rootSpaces: List?, expandedStates: Map, - homeCount: RoomAggregateNotificationCount, - spaceOrderInfo: Map?) { + homeCount: RoomAggregateNotificationCount) { val host = this spaceBetaHeaderItem { id("beta_header") diff --git a/vector/src/main/java/im/vector/app/features/spaces/SpacesListViewModel.kt b/vector/src/main/java/im/vector/app/features/spaces/SpacesListViewModel.kt index 73fcf0a2495..d30d20fdd46 100644 --- a/vector/src/main/java/im/vector/app/features/spaces/SpacesListViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/spaces/SpacesListViewModel.kt @@ -159,15 +159,15 @@ class SpacesListViewModel @AssistedInject constructor(@Assisted initialState: Sp is SpaceListAction.OpenSpaceInvite -> handleSelectSpaceInvite(action) is SpaceListAction.SelectLegacyGroup -> handleSelectGroup(action) is SpaceListAction.MoveSpace -> handleMoveSpace(action) - is SpaceListAction.OnEndDragging -> handleEndDragging(action) - is SpaceListAction.OnStartDragging -> handleStartDragging(action) + is SpaceListAction.OnEndDragging -> handleEndDragging() + is SpaceListAction.OnStartDragging -> handleStartDragging() } } // PRIVATE METHODS ***************************************************************************** var preDragExpandedState: Map? = null - private fun handleStartDragging(action: SpaceListAction.OnStartDragging) = withState { state -> + private fun handleStartDragging() = withState { state -> preDragExpandedState = state.expandedStates.toMap() setState { copy( @@ -178,7 +178,7 @@ class SpacesListViewModel @AssistedInject constructor(@Assisted initialState: Sp } } - private fun handleEndDragging(action: SpaceListAction.OnEndDragging) = withState { state -> + private fun handleEndDragging() { // restore expanded state setState { copy(