malliaridis commented on code in PR #3521: URL: https://github.com/apache/solr/pull/3521#discussion_r2307802359
########## solr/ui/src/commonMain/kotlin/org/apache/solr/ui/components/configsets/ConfigsetsComponent.kt: ########## @@ -0,0 +1,87 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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.apache.solr.ui.components.configsets + +import com.arkivanov.decompose.router.stack.ChildStack +import com.arkivanov.decompose.value.Value +import kotlinx.coroutines.flow.StateFlow +import org.apache.solr.ui.components.configsets.data.ListConfigsets +import org.apache.solr.ui.components.configsets.overview.OverviewComponent +import org.apache.solr.ui.views.navigation.configsets.ConfigsetsTab + +/** + * Contract for the Configsets feature. + * + * Responsibilities: + * - Owns tab/navigation state within the Configsets screen. + * - Loads and exposes the list of available configsets. + * - Persists the currently selected tab and configset. + * + * Exposed state: + * - [model]: reactive UI model (configset names, selected tab, selected configset). + * + * Actions: + * - [onSelectTab]: update the active tab. + * - [onSelectConfigset]: change the active configset. + * + * Notes: + * - Scope: feature-local only; app-level navigation is owned by [MainComponent]. + * - State restoration is implementation-defined (e.g., MVIKotlin Store / Decompose StateKeeper). + * + * See also: + * - Decompose (component lifecycle & composition) + * - MVIKotlin (unidirectional data flow) + */ +interface ConfigsetsComponent { + + /** + * Child stack that holds the navigation state. + */ + val childStack: Value<ChildStack<*, Child>> + + /** + * All possible navigation targets (children) within the Configsets feature. + * + * Each child wraps its own component, which holds the state and logic for that screen. Review Comment: That is correct, but we don't have to mention that here (part of common knowledge). ########## solr/ui/src/commonMain/kotlin/org/apache/solr/ui/components/configsets/integration/DefaultConfigsetsConponent.kt: ########## @@ -0,0 +1,127 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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.apache.solr.ui.components.configsets.integration + +import com.arkivanov.decompose.router.stack.ChildStack +import com.arkivanov.decompose.router.stack.StackNavigation +import com.arkivanov.decompose.router.stack.childStack +import com.arkivanov.decompose.value.Value +import com.arkivanov.mvikotlin.core.instancekeeper.getStore +import com.arkivanov.mvikotlin.core.store.StoreFactory +import com.arkivanov.mvikotlin.extensions.coroutines.stateFlow +import io.ktor.client.HttpClient +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.SupervisorJob +import kotlinx.serialization.Serializable +import org.apache.solr.ui.components.configsets.ConfigsetsComponent +import org.apache.solr.ui.components.configsets.ConfigsetsComponent.Child +import org.apache.solr.ui.components.configsets.overview.OverviewComponent +import org.apache.solr.ui.components.configsets.overview.integration.DefaultOverviewComponent +import org.apache.solr.ui.components.configsets.store.ConfigsetsStore +import org.apache.solr.ui.components.configsets.store.ConfigsetsStoreProvider +import org.apache.solr.ui.utils.AppComponentContext +import org.apache.solr.ui.utils.coroutineScope +import org.apache.solr.ui.utils.map +import org.apache.solr.ui.views.navigation.configsets.ConfigsetsTab + +/** + * Default implementation of the [ConfigsetsComponent]. + */ +class DefaultConfigsetsConponent internal constructor( + componentContext: AppComponentContext, + storeFactory: StoreFactory, + httpClient: HttpClient, + destination: String? = null, + private val overviewComponent: (AppComponentContext) -> OverviewComponent, +) : ConfigsetsComponent, + AppComponentContext by componentContext { + + private val navigation = StackNavigation<Configuration>() + private val stack = childStack( + source = navigation, + serializer = Configuration.serializer(), + initialStack = { calculateInitialStack(destination) }, + handleBackButton = true, + childFactory = ::createChild, + ) + + override val childStack: Value<ChildStack<*, Child>> = stack + constructor( + componentContext: AppComponentContext, + storeFactory: StoreFactory, + httpClient: HttpClient, + ) : this ( + componentContext = componentContext, + storeFactory = storeFactory, + httpClient = httpClient, + destination = null, + overviewComponent = { childContext -> + DefaultOverviewComponent( + componentContext = childContext, + storeFactory = storeFactory, + httpClient = httpClient, + ) + }, + ) + + @Serializable + private sealed interface Configuration { + @Serializable + data object Overview : Configuration + } + + private fun createChild( + configuration: Configuration, + componentContext: AppComponentContext, + ): Child = when (configuration) { + Configuration.Overview -> Child.Overview(overviewComponent(componentContext)) + } + + /** + * Calculates the initial stack based on the destination provided. + */ + private fun calculateInitialStack(destination: String?): List<Configuration> = listOf( + when (destination) { + "Overview" -> Configuration.Overview + else -> Configuration.Overview + }, + ) + + private val mainScope = coroutineScope(SupervisorJob() + mainContext) + private val ioScope = coroutineScope(SupervisorJob() + ioContext) + + private val store = instanceKeeper.getStore { + ConfigsetsStoreProvider( + storeFactory = storeFactory, + client = HttpEnvironmentStoreClient(httpClient), + mainContext = mainScope.coroutineContext, + ioContext = ioScope.coroutineContext, + ).provide() + } + + @OptIn(ExperimentalCoroutinesApi::class) + override val model = store.stateFlow.map(mainScope, configsetsStateToModel) + + override fun onSelectTab(tab: ConfigsetsTab) { + store.accept(ConfigsetsStore.Intent.SelectTab(tab)) Review Comment: Feel free to add `import import org.apache.solr.ui.components.configsets.store.ConfigsetsStore.Intent` to reduce the chained words referencing or using Intents of the component's store. ########## solr/ui/src/commonMain/kotlin/org/apache/solr/ui/components/configsets/integration/DefaultConfigsetsConponent.kt: ########## @@ -0,0 +1,127 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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.apache.solr.ui.components.configsets.integration + +import com.arkivanov.decompose.router.stack.ChildStack +import com.arkivanov.decompose.router.stack.StackNavigation +import com.arkivanov.decompose.router.stack.childStack +import com.arkivanov.decompose.value.Value +import com.arkivanov.mvikotlin.core.instancekeeper.getStore +import com.arkivanov.mvikotlin.core.store.StoreFactory +import com.arkivanov.mvikotlin.extensions.coroutines.stateFlow +import io.ktor.client.HttpClient +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.SupervisorJob +import kotlinx.serialization.Serializable +import org.apache.solr.ui.components.configsets.ConfigsetsComponent +import org.apache.solr.ui.components.configsets.ConfigsetsComponent.Child +import org.apache.solr.ui.components.configsets.overview.OverviewComponent +import org.apache.solr.ui.components.configsets.overview.integration.DefaultOverviewComponent +import org.apache.solr.ui.components.configsets.store.ConfigsetsStore +import org.apache.solr.ui.components.configsets.store.ConfigsetsStoreProvider +import org.apache.solr.ui.utils.AppComponentContext +import org.apache.solr.ui.utils.coroutineScope +import org.apache.solr.ui.utils.map +import org.apache.solr.ui.views.navigation.configsets.ConfigsetsTab + +/** + * Default implementation of the [ConfigsetsComponent]. + */ +class DefaultConfigsetsConponent internal constructor( + componentContext: AppComponentContext, + storeFactory: StoreFactory, + httpClient: HttpClient, + destination: String? = null, Review Comment: I would go with an enum here, so that we limit the possible destinations to only known values. A string can be anything, but an enum cannot. ########## solr/ui/src/commonMain/kotlin/org/apache/solr/ui/components/configsets/store/ConfigsetsStore.kt: ########## @@ -0,0 +1,44 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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.apache.solr.ui.components.configsets.store + +import com.arkivanov.mvikotlin.core.store.Store +import org.apache.solr.ui.components.configsets.data.ListConfigsets +import org.apache.solr.ui.components.configsets.store.ConfigsetsStore.Intent +import org.apache.solr.ui.components.configsets.store.ConfigsetsStore.State +import org.apache.solr.ui.views.navigation.configsets.ConfigsetsTab + +internal interface ConfigsetsStore : Store<Intent, State, Nothing> { + sealed interface Intent { + /** + * Intent for selecting tab. + */ + data class SelectTab(val tab: ConfigsetsTab) : Intent + + /** + * Intent for fetching configsets from the solr. + */ + data object FetchConfigSets : Intent + + data class SelectConfigSet(val configSetName: String) : Intent + } + data class State( + val selectedTab: ConfigsetsTab = ConfigsetsTab.Overview, + val selectedConfigset: String = "", + val configSets: ListConfigsets = ListConfigsets(), Review Comment: You are exposing here an API model too far into the app. It would be better to use an internal domain model instead, like `List<ConfigSet>` where Configset may be a data model with just a `name` property for now (later we may consider using IDs, display names or just names, and other values). Because we are free to use whatever models we want in the frontend, we don't have to keep the API structure across the app, and it will also be easier to migrate later to v2 endpoints (in case v1 is used) if we just map it as soon as we retrieve the data (so at the store client implementation). Also, using a List directly will allow us later to directly use the field for iterating over it (`configSets.forEach { configset -> ... }`) ########## solr/ui/src/commonMain/kotlin/org/apache/solr/ui/views/configsets/ConfigsetsNavBarComponent.kt: ########## @@ -0,0 +1,146 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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. + */ +@file:OptIn(ExperimentalMaterial3Api::class) + +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.widthIn +import androidx.compose.material3.DropdownMenuItem +import androidx.compose.material3.ExperimentalMaterial3Api +import androidx.compose.material3.ExposedDropdownMenuBox +import androidx.compose.material3.ExposedDropdownMenuDefaults +import androidx.compose.material3.HorizontalDivider +import androidx.compose.material3.OutlinedTextField +import androidx.compose.material3.ScrollableTabRow +import androidx.compose.material3.Tab +import androidx.compose.material3.TabRowDefaults +import androidx.compose.material3.TabRowDefaults.tabIndicatorOffset +import androidx.compose.material3.Text +import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.text.style.TextOverflow +import androidx.compose.ui.unit.dp +import org.apache.solr.ui.components.configsets.data.ListConfigsets +import org.apache.solr.ui.generated.resources.Res +import org.apache.solr.ui.generated.resources.configsets_files +import org.apache.solr.ui.generated.resources.configsets_index_query +import org.apache.solr.ui.generated.resources.configsets_overview +import org.apache.solr.ui.generated.resources.configsets_request_handlers +import org.apache.solr.ui.generated.resources.configsets_schema +import org.apache.solr.ui.generated.resources.configsets_search_components +import org.apache.solr.ui.generated.resources.configsets_update_configuration +import org.apache.solr.ui.views.navigation.configsets.ConfigsetsTab +import org.jetbrains.compose.resources.stringResource + +@Composable +fun ConfigsetsNavBarComponent( + selectedTab: ConfigsetsTab, + selectTab: (ConfigsetsTab) -> Unit, + selectedConfigSet: String, + selectConfigset: (String) -> Unit, + availableConfigsets: ListConfigsets, + modifier: Modifier = Modifier, + content: @Composable (tab: ConfigsetsTab, configset: String) -> Unit = { tab, _ -> + Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { Text(stringResource(tabLabelRes(tab))) } + }, Review Comment: This is now the case where I would ask you "why does a navigation bar that is used for navigation need to know configsets? And why does it have a content? What can I put into it, and where is it added?" By definition, I would expect the navigation bar to be only the elements on the top that I can choose from, so the navigation items. You are now providing the content as well, which kinda confuses the user of this composable. At the same time, I am wondering, does the navigation bar need to know about the configsets? I am just navigating through tabs, am I not? ########## solr/ui/src/commonMain/kotlin/org/apache/solr/ui/views/configsets/ConfigsetsNavBarComponent.kt: ########## @@ -0,0 +1,146 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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. + */ +@file:OptIn(ExperimentalMaterial3Api::class) + +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.widthIn +import androidx.compose.material3.DropdownMenuItem +import androidx.compose.material3.ExperimentalMaterial3Api +import androidx.compose.material3.ExposedDropdownMenuBox +import androidx.compose.material3.ExposedDropdownMenuDefaults +import androidx.compose.material3.HorizontalDivider +import androidx.compose.material3.OutlinedTextField +import androidx.compose.material3.ScrollableTabRow +import androidx.compose.material3.Tab +import androidx.compose.material3.TabRowDefaults +import androidx.compose.material3.TabRowDefaults.tabIndicatorOffset +import androidx.compose.material3.Text +import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.text.style.TextOverflow +import androidx.compose.ui.unit.dp +import org.apache.solr.ui.components.configsets.data.ListConfigsets +import org.apache.solr.ui.generated.resources.Res +import org.apache.solr.ui.generated.resources.configsets_files +import org.apache.solr.ui.generated.resources.configsets_index_query +import org.apache.solr.ui.generated.resources.configsets_overview +import org.apache.solr.ui.generated.resources.configsets_request_handlers +import org.apache.solr.ui.generated.resources.configsets_schema +import org.apache.solr.ui.generated.resources.configsets_search_components +import org.apache.solr.ui.generated.resources.configsets_update_configuration +import org.apache.solr.ui.views.navigation.configsets.ConfigsetsTab +import org.jetbrains.compose.resources.stringResource + +@Composable +fun ConfigsetsNavBarComponent( + selectedTab: ConfigsetsTab, + selectTab: (ConfigsetsTab) -> Unit, + selectedConfigSet: String, + selectConfigset: (String) -> Unit, + availableConfigsets: ListConfigsets, + modifier: Modifier = Modifier, + content: @Composable (tab: ConfigsetsTab, configset: String) -> Unit = { tab, _ -> + Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { Text(stringResource(tabLabelRes(tab))) } + }, +) { + if (availableConfigsets.names.isEmpty()) { + Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { + Text("No configsets available") + } + return + } + + var expanded by remember { mutableStateOf(false) } + val selectedIndex = ConfigsetsTab.entries.indexOf(selectedTab) + Column(modifier) { + ScrollableTabRow( + selectedTabIndex = selectedIndex, + edgePadding = 16.dp, + divider = { HorizontalDivider(thickness = 1.dp) }, + indicator = { pos -> + TabRowDefaults.SecondaryIndicator( + Modifier.tabIndicatorOffset(pos[selectedIndex]), + ) + }, + ) { + ConfigsetsTab.entries.forEach { tab -> + Tab( + selected = selectedTab == tab, + onClick = { selectTab(tab) }, + text = { Text(stringResource(tabLabelRes(tab)), maxLines = 1, overflow = TextOverflow.Ellipsis) }, + ) + } + } + + Row( + modifier = Modifier + .fillMaxWidth() + .padding(horizontal = 16.dp, vertical = 8.dp), + verticalAlignment = Alignment.CenterVertically, + ) { + ExposedDropdownMenuBox( + expanded = expanded, + onExpandedChange = { expanded = !expanded }, + modifier = Modifier.widthIn(min = 256.dp).weight(1f), + ) { + OutlinedTextField( + value = selectedConfigSet, + onValueChange = {}, + readOnly = true, + label = { Text("Configset") }, + trailingIcon = { ExposedDropdownMenuDefaults.TrailingIcon(expanded) }, + modifier = Modifier.menuAnchor().fillMaxWidth(), + ) + ExposedDropdownMenu(expanded = expanded, onDismissRequest = { expanded = false }) { + availableConfigsets.names.forEach { name -> + DropdownMenuItem( + text = { Text(name) }, + onClick = { + selectConfigset(name) + expanded = false + }, + ) + } + } + } + } Review Comment: This block seems like it does not belong here 🤔 ########## solr/ui/src/commonMain/kotlin/org/apache/solr/ui/components/configsets/data/ListConfigsets.kt: ########## @@ -0,0 +1,29 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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.apache.solr.ui.components.configsets.data + +import kotlinx.serialization.SerialName +import kotlinx.serialization.Serializable + +/** + * List of config sets. + */ +@Serializable +data class ListConfigsets( + @SerialName("configSets") + val names: List<String> = emptyList(), Review Comment: Is there a special reason you chose to explicitly define a serial name, rather than using `val configSets`? ########## solr/ui/src/commonMain/kotlin/org/apache/solr/ui/components/configsets/ConfigsetsComponent.kt: ########## @@ -0,0 +1,87 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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.apache.solr.ui.components.configsets + +import com.arkivanov.decompose.router.stack.ChildStack +import com.arkivanov.decompose.value.Value +import kotlinx.coroutines.flow.StateFlow +import org.apache.solr.ui.components.configsets.data.ListConfigsets +import org.apache.solr.ui.components.configsets.overview.OverviewComponent +import org.apache.solr.ui.views.navigation.configsets.ConfigsetsTab + +/** + * Contract for the Configsets feature. + * + * Responsibilities: + * - Owns tab/navigation state within the Configsets screen. + * - Loads and exposes the list of available configsets. + * - Persists the currently selected tab and configset. Review Comment: Point 1 and 3 is the same I believe? Point 2 is responsibility eventually out of scope (not necessarily though). ########## solr/ui/src/commonMain/kotlin/org/apache/solr/ui/components/configsets/integration/HttpEnvironmentStoreClient.kt: ########## @@ -0,0 +1,47 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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.apache.solr.ui.components.configsets.integration + +import io.ktor.client.HttpClient +import io.ktor.client.call.body +import io.ktor.client.request.get +import io.ktor.http.isSuccess +import org.apache.solr.ui.components.configsets.data.ListConfigsets +import org.apache.solr.ui.components.configsets.store.ConfigsetsStoreProvider + +/** + * Client implementation of the [ConfigsetsStoreProvider.Client] that makes use + * of a preconfigured HTTP client for accessing the Solr API. + * + * @property httpClient HTTP client to use for accessing the API. The client has to be + * configured with a default request that includes the host, port and schema. The client + * should also include the necessary authentication data if authentication / authorization + * is enabled. + */ +class HttpEnvironmentStoreClient( Review Comment: I believe you meant here `HttpConfigStoreClient`? ########## solr/ui/src/commonMain/kotlin/org/apache/solr/ui/components/configsets/store/ConfigsetsStore.kt: ########## @@ -0,0 +1,44 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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.apache.solr.ui.components.configsets.store + +import com.arkivanov.mvikotlin.core.store.Store +import org.apache.solr.ui.components.configsets.data.ListConfigsets +import org.apache.solr.ui.components.configsets.store.ConfigsetsStore.Intent +import org.apache.solr.ui.components.configsets.store.ConfigsetsStore.State +import org.apache.solr.ui.views.navigation.configsets.ConfigsetsTab + +internal interface ConfigsetsStore : Store<Intent, State, Nothing> { + sealed interface Intent { + /** + * Intent for selecting tab. + */ + data class SelectTab(val tab: ConfigsetsTab) : Intent + + /** + * Intent for fetching configsets from the solr. + */ + data object FetchConfigSets : Intent Review Comment: Can the user manually fetch the configsets? Because intents describe a user's intent / wish to do something (on request). If not, consider using only `Action. FetchInitialConfigsets` in `ConfigsetsStoreProvider` (for initial fetch) and removing this. ########## solr/ui/src/commonMain/kotlin/org/apache/solr/ui/views/configsets/ConfigsetsContent.kt: ########## @@ -0,0 +1,80 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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.apache.solr.ui.views.configsets + +import ConfigsetsNavBarComponent +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.ExperimentalLayoutApi +import androidx.compose.foundation.layout.FlowRow +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.material3.Text +import androidx.compose.runtime.Composable +import androidx.compose.runtime.collectAsState +import androidx.compose.runtime.getValue +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.unit.dp +import com.arkivanov.decompose.extensions.compose.subscribeAsState +import org.apache.solr.ui.components.configsets.ConfigsetsComponent +import org.apache.solr.ui.components.configsets.ConfigsetsComponent.Child +import org.apache.solr.ui.views.navigation.configsets.ConfigsetsTab + + +@OptIn(ExperimentalLayoutApi::class) +@Composable +fun ConfigsetsContent( + component: ConfigsetsComponent, + modifier: Modifier = Modifier, +) = FlowRow( + modifier = modifier, + horizontalArrangement = Arrangement.spacedBy(16.dp), + verticalArrangement = Arrangement.spacedBy(16.dp), +) { + val model by component.model.collectAsState() + + // Decompose child to access OverviewComponent + val stack by component.childStack.subscribeAsState() + val currentChild = stack.active.instance + + ConfigsetsNavBarComponent( + selectedTab = model.selectedTab, + selectTab = { tab: ConfigsetsTab -> component.onSelectTab(tab) }, + selectedConfigSet = model.selectedConfigset, + selectConfigset = { s: String -> component.onSelectConfigset(s) }, + availableConfigsets = model.configSets, + content = { tab, _ -> + when (tab) { + ConfigsetsTab.Overview -> { + when (val child = currentChild) { + is Child.Overview -> + OverviewContent(component = child.component) + else -> + Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { Review Comment: Always go with or without names for the parameters, avoid mixing it. Consider also writing params in new lines if more than 2. ########## solr/ui/src/commonMain/kotlin/org/apache/solr/ui/components/configsets/store/ConfigsetsStoreProvider.kt: ########## @@ -0,0 +1,110 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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.apache.solr.ui.components.configsets.store + +import com.arkivanov.mvikotlin.core.store.Reducer +import com.arkivanov.mvikotlin.core.store.SimpleBootstrapper +import com.arkivanov.mvikotlin.core.store.Store +import com.arkivanov.mvikotlin.core.store.StoreFactory +import com.arkivanov.mvikotlin.extensions.coroutines.CoroutineExecutor +import kotlin.coroutines.CoroutineContext +import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext +import org.apache.solr.ui.components.configsets.data.ListConfigsets +import org.apache.solr.ui.components.configsets.store.ConfigsetsStore.Intent +import org.apache.solr.ui.components.configsets.store.ConfigsetsStore.State +import org.apache.solr.ui.views.navigation.configsets.ConfigsetsTab + +/** + * Store provider that [provide]s instances of [ConfigsetsStore]. + * + * @property storeFactory Store factory to use for creating the store. + * @property client Client implementation to use for resolving [Intent]s and [Action]s. + * @property ioContext Coroutine context used for IO activity. + */ +internal class ConfigsetsStoreProvider( + private val storeFactory: StoreFactory, + private val client: Client, + private val mainContext: CoroutineContext, + private val ioContext: CoroutineContext, +) { + + fun provide(): ConfigsetsStore = object : + ConfigsetsStore, + Store<Intent, State, Nothing> by storeFactory.create( + name = "ConfigsetsStore", + initialState = State(), + bootstrapper = SimpleBootstrapper(Action.FetchInitialConfigsets), + executorFactory = ::ExecutorImpl, + reducer = ReducerImpl, + ) {} + + private sealed interface Action { + /** + * Action used for initiating the initial fetch of configsets data. + */ + data object FetchInitialConfigsets : Action + } + + private sealed interface Message { + data class ConfigSetsUpdated(val configsets: ListConfigsets) : Message + data class SelectedTabChanged(val tab: ConfigsetsTab) : Message + data class SelectedConfigSetChanged(val configsetName: String) : Message + } + + private inner class ExecutorImpl : CoroutineExecutor<Intent, Action, State, Message, Nothing>(mainContext) { + override fun executeAction(action: Action) = when (action) { + Action.FetchInitialConfigsets -> { + fetchConfigSets() + } + } + override fun executeIntent(intent: Intent) = when (intent) { + is Intent.FetchConfigSets -> fetchConfigSets() + is Intent.SelectTab -> dispatch(Message.SelectedTabChanged(intent.tab)) + is Intent.SelectConfigSet -> dispatch(Message.SelectedConfigSetChanged(intent.configSetName)) + } + private fun fetchConfigSets() { + scope.launch { + withContext(ioContext) { + client.fetchConfigSets() + }.onSuccess { sets -> + dispatch(Message.ConfigSetsUpdated(sets)) + } + } + } + } + + /** + * Reducer implementation that consumes [Message]s and updates the store's [State]. + */ + private object ReducerImpl : Reducer<State, Message> { + override fun State.reduce(msg: Message): State = when (msg) { + is Message.ConfigSetsUpdated -> copy(configSets = msg.configsets) + is Message.SelectedTabChanged -> copy(selectedTab = msg.tab) + is Message.SelectedConfigSetChanged -> copy(selectedConfigset = msg.configsetName) + } + } + + /** + * Client interface for fetching configsets information. + */ + interface Client { + /** To fetch a list of configsets. */ + suspend fun fetchConfigSets(): Result<ListConfigsets> Review Comment: So I would recommend not to return a `ListConfigsets` (API model), but rather a `List<Configset>` (domain model) here. ########## solr/ui/src/commonMain/kotlin/org/apache/solr/ui/components/configsets/ConfigsetsComponent.kt: ########## @@ -0,0 +1,87 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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.apache.solr.ui.components.configsets + +import com.arkivanov.decompose.router.stack.ChildStack +import com.arkivanov.decompose.value.Value +import kotlinx.coroutines.flow.StateFlow +import org.apache.solr.ui.components.configsets.data.ListConfigsets +import org.apache.solr.ui.components.configsets.overview.OverviewComponent +import org.apache.solr.ui.views.navigation.configsets.ConfigsetsTab + +/** + * Contract for the Configsets feature. + * + * Responsibilities: + * - Owns tab/navigation state within the Configsets screen. + * - Loads and exposes the list of available configsets. + * - Persists the currently selected tab and configset. + * + * Exposed state: + * - [model]: reactive UI model (configset names, selected tab, selected configset). + * + * Actions: + * - [onSelectTab]: update the active tab. + * - [onSelectConfigset]: change the active configset. + * + * Notes: + * - Scope: feature-local only; app-level navigation is owned by [MainComponent]. + * - State restoration is implementation-defined (e.g., MVIKotlin Store / Decompose StateKeeper). + * + * See also: + * - Decompose (component lifecycle & composition) + * - MVIKotlin (unidirectional data flow) + */ +interface ConfigsetsComponent { + + /** + * Child stack that holds the navigation state. + */ + val childStack: Value<ChildStack<*, Child>> + + /** + * All possible navigation targets (children) within the Configsets feature. + * + * Each child wraps its own component, which holds the state and logic for that screen. + */ + sealed interface Child { + data class Overview(val component: OverviewComponent) : Child + } + + /** + * UI model for the Configsets screen. + * + * @property selectedTab Active tab. + * @property configSets List of available configsets (domain model). + * @property selectedConfigset Name of the active configset (non-null when available). Review Comment: Is it perhaps better to allow null values, so that - `null` means no selection (no reference, no configset) - `string` means selection (actual configset exists) Of course an empty string is also not a valid configset, but from reading the field I believe `selectedConfigset: String? = null` would be easier to handle. Since in Kotlin you have explicit nullability, you can handle it differently like with `value?.let { ... }` (a Kotlin feature useful for executing code in a scoped block only if a value is not null). ########## solr/ui/src/commonMain/kotlin/org/apache/solr/ui/views/configsets/ConfigsetsNavBarComponent.kt: ########## @@ -0,0 +1,146 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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. + */ +@file:OptIn(ExperimentalMaterial3Api::class) + +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.widthIn +import androidx.compose.material3.DropdownMenuItem +import androidx.compose.material3.ExperimentalMaterial3Api +import androidx.compose.material3.ExposedDropdownMenuBox +import androidx.compose.material3.ExposedDropdownMenuDefaults +import androidx.compose.material3.HorizontalDivider +import androidx.compose.material3.OutlinedTextField +import androidx.compose.material3.ScrollableTabRow +import androidx.compose.material3.Tab +import androidx.compose.material3.TabRowDefaults +import androidx.compose.material3.TabRowDefaults.tabIndicatorOffset +import androidx.compose.material3.Text +import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.text.style.TextOverflow +import androidx.compose.ui.unit.dp +import org.apache.solr.ui.components.configsets.data.ListConfigsets +import org.apache.solr.ui.generated.resources.Res +import org.apache.solr.ui.generated.resources.configsets_files +import org.apache.solr.ui.generated.resources.configsets_index_query +import org.apache.solr.ui.generated.resources.configsets_overview +import org.apache.solr.ui.generated.resources.configsets_request_handlers +import org.apache.solr.ui.generated.resources.configsets_schema +import org.apache.solr.ui.generated.resources.configsets_search_components +import org.apache.solr.ui.generated.resources.configsets_update_configuration +import org.apache.solr.ui.views.navigation.configsets.ConfigsetsTab +import org.jetbrains.compose.resources.stringResource + +@Composable +fun ConfigsetsNavBarComponent( Review Comment: Since it is a plain composable function, avoid using the kinda "reserved" suffix "Component". This avoids confusion with the component interfaces and implementations. Just name it `ConfigsetsNavBar` for example. ########## solr/ui/src/commonMain/kotlin/org/apache/solr/ui/components/configsets/integration/DefaultConfigsetsConponent.kt: ########## @@ -0,0 +1,127 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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.apache.solr.ui.components.configsets.integration + +import com.arkivanov.decompose.router.stack.ChildStack +import com.arkivanov.decompose.router.stack.StackNavigation +import com.arkivanov.decompose.router.stack.childStack +import com.arkivanov.decompose.value.Value +import com.arkivanov.mvikotlin.core.instancekeeper.getStore +import com.arkivanov.mvikotlin.core.store.StoreFactory +import com.arkivanov.mvikotlin.extensions.coroutines.stateFlow +import io.ktor.client.HttpClient +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.SupervisorJob +import kotlinx.serialization.Serializable +import org.apache.solr.ui.components.configsets.ConfigsetsComponent +import org.apache.solr.ui.components.configsets.ConfigsetsComponent.Child +import org.apache.solr.ui.components.configsets.overview.OverviewComponent +import org.apache.solr.ui.components.configsets.overview.integration.DefaultOverviewComponent +import org.apache.solr.ui.components.configsets.store.ConfigsetsStore +import org.apache.solr.ui.components.configsets.store.ConfigsetsStoreProvider +import org.apache.solr.ui.utils.AppComponentContext +import org.apache.solr.ui.utils.coroutineScope +import org.apache.solr.ui.utils.map +import org.apache.solr.ui.views.navigation.configsets.ConfigsetsTab + +/** + * Default implementation of the [ConfigsetsComponent]. + */ +class DefaultConfigsetsConponent internal constructor( + componentContext: AppComponentContext, + storeFactory: StoreFactory, + httpClient: HttpClient, + destination: String? = null, + private val overviewComponent: (AppComponentContext) -> OverviewComponent, +) : ConfigsetsComponent, + AppComponentContext by componentContext { + + private val navigation = StackNavigation<Configuration>() + private val stack = childStack( + source = navigation, + serializer = Configuration.serializer(), + initialStack = { calculateInitialStack(destination) }, + handleBackButton = true, + childFactory = ::createChild, + ) + + override val childStack: Value<ChildStack<*, Child>> = stack + constructor( + componentContext: AppComponentContext, + storeFactory: StoreFactory, + httpClient: HttpClient, + ) : this ( + componentContext = componentContext, + storeFactory = storeFactory, + httpClient = httpClient, + destination = null, + overviewComponent = { childContext -> + DefaultOverviewComponent( + componentContext = childContext, + storeFactory = storeFactory, + httpClient = httpClient, + ) + }, + ) + + @Serializable + private sealed interface Configuration { + @Serializable + data object Overview : Configuration + } + + private fun createChild( + configuration: Configuration, + componentContext: AppComponentContext, + ): Child = when (configuration) { + Configuration.Overview -> Child.Overview(overviewComponent(componentContext)) + } + + /** + * Calculates the initial stack based on the destination provided. + */ + private fun calculateInitialStack(destination: String?): List<Configuration> = listOf( + when (destination) { + "Overview" -> Configuration.Overview Review Comment: Consider enums (limited possibilities) instead of strings, this way, no else case is needed (thanks to type-safety). ########## solr/ui/src/commonMain/kotlin/org/apache/solr/ui/views/configsets/ConfigsetsNavBarComponent.kt: ########## @@ -0,0 +1,146 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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. + */ +@file:OptIn(ExperimentalMaterial3Api::class) + +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.widthIn +import androidx.compose.material3.DropdownMenuItem +import androidx.compose.material3.ExperimentalMaterial3Api +import androidx.compose.material3.ExposedDropdownMenuBox +import androidx.compose.material3.ExposedDropdownMenuDefaults +import androidx.compose.material3.HorizontalDivider +import androidx.compose.material3.OutlinedTextField +import androidx.compose.material3.ScrollableTabRow +import androidx.compose.material3.Tab +import androidx.compose.material3.TabRowDefaults +import androidx.compose.material3.TabRowDefaults.tabIndicatorOffset +import androidx.compose.material3.Text +import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.text.style.TextOverflow +import androidx.compose.ui.unit.dp +import org.apache.solr.ui.components.configsets.data.ListConfigsets +import org.apache.solr.ui.generated.resources.Res +import org.apache.solr.ui.generated.resources.configsets_files +import org.apache.solr.ui.generated.resources.configsets_index_query +import org.apache.solr.ui.generated.resources.configsets_overview +import org.apache.solr.ui.generated.resources.configsets_request_handlers +import org.apache.solr.ui.generated.resources.configsets_schema +import org.apache.solr.ui.generated.resources.configsets_search_components +import org.apache.solr.ui.generated.resources.configsets_update_configuration +import org.apache.solr.ui.views.navigation.configsets.ConfigsetsTab +import org.jetbrains.compose.resources.stringResource + +@Composable +fun ConfigsetsNavBarComponent( + selectedTab: ConfigsetsTab, + selectTab: (ConfigsetsTab) -> Unit, + selectedConfigSet: String, + selectConfigset: (String) -> Unit, + availableConfigsets: ListConfigsets, + modifier: Modifier = Modifier, + content: @Composable (tab: ConfigsetsTab, configset: String) -> Unit = { tab, _ -> + Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { Text(stringResource(tabLabelRes(tab))) } + }, +) { + if (availableConfigsets.names.isEmpty()) { + Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { + Text("No configsets available") + } + return + } + + var expanded by remember { mutableStateOf(false) } Review Comment: Another UI state that should be added to a component's state. If you do not add it, you won't be able later to load additional information on demand once the user expands it. 🤔 ########## solr/ui/src/commonMain/composeResources/values/strings.xml: ########## @@ -50,6 +50,16 @@ <string name="nav_security">Security</string> <string name="nav_thread_dump">Thread Dump</string> + <!-- Configsets (nav) --> + <string name="configsets_overview">Overview</string> + <string name="configsets_libraries">Libraries</string> + <string name="configsets_files">Files</string> + <string name="configsets_schema">Schema</string> + <string name="configsets_update_configuration">Update Configuration</string> + <string name="configsets_index_query">Index / Query</string> + <string name="configsets_request_handlers">Request Handlers / Dispatchers</string> + <string name="configsets_search_components">Search Components</string> Review Comment: I believe that we could use general terms here, without prefixing it with `configsets`, because we will likely have similar words (like "overview", "files", "schema") in other places too, so reusing them would be beneficial (smaller files, less duplication). ########## solr/ui/src/commonMain/kotlin/org/apache/solr/ui/components/configsets/ConfigsetsComponent.kt: ########## @@ -0,0 +1,87 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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.apache.solr.ui.components.configsets + +import com.arkivanov.decompose.router.stack.ChildStack +import com.arkivanov.decompose.value.Value +import kotlinx.coroutines.flow.StateFlow +import org.apache.solr.ui.components.configsets.data.ListConfigsets +import org.apache.solr.ui.components.configsets.overview.OverviewComponent +import org.apache.solr.ui.views.navigation.configsets.ConfigsetsTab + +/** + * Contract for the Configsets feature. + * + * Responsibilities: + * - Owns tab/navigation state within the Configsets screen. + * - Loads and exposes the list of available configsets. + * - Persists the currently selected tab and configset. + * + * Exposed state: + * - [model]: reactive UI model (configset names, selected tab, selected configset). + * + * Actions: + * - [onSelectTab]: update the active tab. + * - [onSelectConfigset]: change the active configset. + * + * Notes: + * - Scope: feature-local only; app-level navigation is owned by [MainComponent]. + * - State restoration is implementation-defined (e.g., MVIKotlin Store / Decompose StateKeeper). + * + * See also: + * - Decompose (component lifecycle & composition) + * - MVIKotlin (unidirectional data flow) Review Comment: No need to repeat yourself here, because readers can look into the interface. When updating model or actions, you would also have to update documentation here (can be avoided). Nor do you have to provide information that is "common" knowledge or out of scope of this configset ("see also" points). You can reduce it to only what this component is all about, "managing the navigation and content / layout of the configset screen". ########## solr/ui/src/commonMain/kotlin/org/apache/solr/ui/components/configsets/overview/OverviewComponent.kt: ########## @@ -0,0 +1,19 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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.apache.solr.ui.components.configsets.overview + +interface OverviewComponent Review Comment: I would go with `ConfigsetOverviewComponent` to avoid naming conflicts later on when other sub-sections have an "overview" component. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
