From 8585145a92c74d362c84175d9bba59fc39911d18 Mon Sep 17 00:00:00 2001 From: "andras.maczak" Date: Thu, 6 Nov 2025 20:08:38 +0100 Subject: [PATCH 01/33] some progress on notebook list --- .../features/notebook/NotebookScreen.kt | 180 +++++++++------- .../common/composable/NotebookAppBar.kt | 37 +++- .../notebook/common/model/NotebookType.kt | 2 +- .../horizonui/foundation/HorizonColors.kt | 2 + .../horizonui/molecules/DropdownChip.kt | 193 ++++++++++++++++++ libs/horizon/src/main/res/values/colors.xml | 1 + libs/horizon/src/main/res/values/strings.xml | 5 +- 7 files changed, 333 insertions(+), 87 deletions(-) create mode 100644 libs/horizon/src/main/java/com/instructure/horizon/horizonui/molecules/DropdownChip.kt diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt index fdf3723dae..ef32291e32 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt @@ -23,6 +23,7 @@ import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.PaddingValues 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.lazy.LazyColumn @@ -31,8 +32,11 @@ import androidx.compose.foundation.lazy.rememberLazyListState import androidx.compose.material3.Scaffold import androidx.compose.material3.Text import androidx.compose.runtime.Composable +import androidx.compose.runtime.remember import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.draw.clip +import androidx.compose.ui.graphics.Color import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.res.stringResource import androidx.compose.ui.text.style.TextOverflow @@ -48,7 +52,6 @@ import com.instructure.horizon.R import com.instructure.horizon.features.notebook.common.composable.NotebookAppBar import com.instructure.horizon.features.notebook.common.composable.NotebookHighlightedText import com.instructure.horizon.features.notebook.common.composable.NotebookPill -import com.instructure.horizon.features.notebook.common.composable.NotebookTypeSelect import com.instructure.horizon.features.notebook.common.model.Note import com.instructure.horizon.features.notebook.common.model.NotebookType import com.instructure.horizon.horizonui.foundation.HorizonColors @@ -58,6 +61,10 @@ import com.instructure.horizon.horizonui.foundation.HorizonSpace import com.instructure.horizon.horizonui.foundation.HorizonTypography import com.instructure.horizon.horizonui.foundation.SpaceSize import com.instructure.horizon.horizonui.foundation.horizonShadow +import com.instructure.horizon.horizonui.molecules.DropdownChip +import com.instructure.horizon.horizonui.molecules.DropdownItem +import com.instructure.horizon.horizonui.molecules.FilterDropdownChip +import com.instructure.horizon.horizonui.molecules.FilterDropdownItem import com.instructure.horizon.horizonui.molecules.IconButton import com.instructure.horizon.horizonui.molecules.IconButtonColor import com.instructure.horizon.horizonui.molecules.IconButtonSize @@ -99,73 +106,67 @@ fun NotebookScreen( } }, ) { padding -> - LazyColumn( - state = scrollState, + Box( + contentAlignment = Alignment.TopCenter, modifier = Modifier - .padding(padding), - contentPadding = PaddingValues(24.dp) + .fillMaxSize() + .padding(paddingValues = padding) + .clip(HorizonCornerRadius.level5) + .background(HorizonColors.Surface.pageSecondary()) ) { - if (state.showFilters && state.notes.isNotEmpty()) { - item { - FilterContent( - state.selectedFilter, - state.onFilterSelected - ) - } - } - - if (state.notes.isNotEmpty()){ - item { - Column { - Text( - text = stringResource(R.string.notebookNotesLabel), - style = HorizonTypography.labelLargeBold, - color = HorizonColors.Text.title() + LazyColumn( + state = scrollState, + modifier = Modifier, + contentPadding = PaddingValues(horizontal = 16.dp, vertical = 24.dp) + ) { + if (state.showFilters && state.notes.isNotEmpty()) { + item { + FilterContent( + state.selectedFilter, + state.onFilterSelected ) - - HorizonSpace(SpaceSize.SPACE_12) } } - } - if (state.isLoading) { - item { - LoadingContent() - } - } else if (state.notes.isEmpty()) { - item { - EmptyContent() - } - } else { - items(state.notes) { note -> - Column { - NoteContent(note) { - onNoteSelected?.invoke(note) ?: mainNavController.navigate( - MainNavigationRoute.ModuleItemSequence( - courseId = note.courseId, - moduleItemAssetType = note.objectType.value, - moduleItemAssetId = note.objectId, + if (state.isLoading) { + item { + LoadingContent() + } + } else if (state.notes.isEmpty()) { + item { + EmptyContent() + } + } else { + items(state.notes) { note -> + Column { + NoteContent(note) { + onNoteSelected?.invoke(note) ?: mainNavController.navigate( + MainNavigationRoute.ModuleItemSequence( + courseId = note.courseId, + moduleItemAssetType = note.objectType.value, + moduleItemAssetId = note.objectId, + ) ) - ) - } + } - if (state.notes.lastOrNull() != note) { - HorizonSpace(SpaceSize.SPACE_12) + if (state.notes.lastOrNull() != note) { + HorizonSpace(SpaceSize.SPACE_4) + } } } - } - item { - Column { - HorizonSpace(SpaceSize.SPACE_24) + item { + Column { + HorizonSpace(SpaceSize.SPACE_24) - NotesPager( - canNavigateBack = state.hasPreviousPage, - canNavigateForward = state.hasNextPage, - isLoading = state.isLoading, - onNavigateBack = state.loadPreviousPage, - onNavigateForward = state.loadNextPage - ) + NotesPager( + canNavigateBack = state.hasPreviousPage, + canNavigateForward = state.hasNextPage, + isLoading = state.isLoading, + onNavigateBack = state.loadPreviousPage, + onNavigateForward = state.loadNextPage + ) + } } } } @@ -178,34 +179,63 @@ private fun FilterContent( selectedFilter: NotebookType?, onFilterSelected: (NotebookType?) -> Unit, ) { - Column { - Text( - text = stringResource(R.string.notebookFilterLabel), - style = HorizonTypography.labelLargeBold, - color = HorizonColors.Text.title() - ) + val context = LocalContext.current - HorizonSpace(SpaceSize.SPACE_12) + val allNotesItem = DropdownItem( + value = null as NotebookType?, + label = context.getString(R.string.notebookTypeAllNotes), + iconRes = R.drawable.menu, + iconTint = HorizonColors.Icon.default() + ) + val importantColor = HorizonColors.PrimitivesSea.sea12() + val confusingColor = HorizonColors.PrimitivesRed.red12() - Row { - NotebookTypeSelect( - type = NotebookType.Important, - isSelected = selectedFilter == NotebookType.Important, - onSelect = { onFilterSelected(if (selectedFilter == NotebookType.Important) null else NotebookType.Important) }, - modifier = Modifier.weight(1f) + val typeItems = remember { + + listOf( + allNotesItem, + DropdownItem( + value = NotebookType.Important, + label = context.getString(NotebookType.Important.labelRes), + iconRes = NotebookType.Important.iconRes, + iconTint = context.getColor(NotebookType.Important.color).let { Color(it) }, + backgroundColor = importantColor + ), + DropdownItem( + value = NotebookType.Confusing, + label = context.getString(NotebookType.Confusing.labelRes), + iconRes = NotebookType.Confusing.iconRes, + iconTint = context.getColor(NotebookType.Confusing.color).let { androidx.compose.ui.graphics.Color(it) }, + backgroundColor = confusingColor ) + ) + } - HorizonSpace(SpaceSize.SPACE_12) + val selectedTypeItem = if (selectedFilter == null) allNotesItem else typeItems.find { it.value == selectedFilter } + + Column { + Row( + modifier = Modifier.fillMaxWidth(), + horizontalArrangement = Arrangement.spacedBy(8.dp) + ) { + /*FilterDropdownChip( + items = emptyList(), + selectedItem = null, + onItemSelected = { }, + placeholder = stringResource(R.string.notebookFilterCoursePlaceholder), + modifier = Modifier.weight(1f) + )*/ - NotebookTypeSelect( - type = NotebookType.Confusing, - isSelected = selectedFilter == NotebookType.Confusing, - onSelect = { onFilterSelected(if (selectedFilter == NotebookType.Confusing) null else NotebookType.Confusing) }, + DropdownChip( + items = typeItems, + selectedItem = selectedTypeItem, + onItemSelected = { item -> onFilterSelected(item?.value) }, + placeholder = stringResource(R.string.notebookFilterTypePlaceholder), modifier = Modifier.weight(1f) ) } - HorizonSpace(SpaceSize.SPACE_24) + HorizonSpace(SpaceSize.SPACE_16) } } diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/common/composable/NotebookAppBar.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/common/composable/NotebookAppBar.kt index 8f042626c8..6b3284077e 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/common/composable/NotebookAppBar.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/common/composable/NotebookAppBar.kt @@ -16,14 +16,19 @@ */ package com.instructure.horizon.features.notebook.common.composable +import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.padding -import androidx.compose.material3.CenterAlignedTopAppBar +import androidx.compose.foundation.layout.size import androidx.compose.material3.ExperimentalMaterial3Api +import androidx.compose.material3.Icon import androidx.compose.material3.Text +import androidx.compose.material3.TopAppBar import androidx.compose.material3.TopAppBarDefaults import androidx.compose.runtime.Composable +import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.platform.LocalContext +import androidx.compose.ui.res.painterResource import androidx.compose.ui.res.stringResource import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.unit.dp @@ -43,13 +48,25 @@ fun NotebookAppBar( navigateBack: (() -> Unit)? = null, onClose: (() -> Unit)? = null ) { - CenterAlignedTopAppBar( + TopAppBar( title = { - Text( - text = stringResource(R.string.notebookTitle), - style = HorizonTypography.h3, - color = HorizonColors.Text.title() - ) + Row( + verticalAlignment = Alignment.CenterVertically + ) { + Icon( + painter = painterResource(R.drawable.edit_note), + contentDescription = null, + tint = HorizonColors.Icon.default(), + modifier = Modifier + .size(20.dp) + .padding(end = 4.dp) + ) + Text( + text = stringResource(R.string.notebookTitle), + style = HorizonTypography.h4, + color = HorizonColors.Text.title() + ) + } }, navigationIcon = { if (navigateBack != null) { @@ -60,7 +77,7 @@ fun NotebookAppBar( size = IconButtonSize.SMALL, elevation = HorizonElevation.level4, onClick = navigateBack, - modifier = Modifier.padding(horizontal = 24.dp) + modifier = Modifier.padding(horizontal = 16.dp) ) } }, @@ -68,12 +85,12 @@ fun NotebookAppBar( if (onClose != null) { IconButton( iconRes = R.drawable.close, - contentDescription = stringResource(R.string.a11yNavigateBack), + contentDescription = stringResource(R.string.a11y_close), color = IconButtonColor.Inverse, size = IconButtonSize.SMALL, elevation = HorizonElevation.level4, onClick = onClose, - modifier = Modifier.padding(horizontal = 24.dp) + modifier = Modifier.padding(horizontal = 16.dp) ) } }, diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/common/model/NotebookType.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/common/model/NotebookType.kt index 81783b6315..ba0a59818a 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/common/model/NotebookType.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/common/model/NotebookType.kt @@ -11,5 +11,5 @@ enum class NotebookType( @ColorRes val color: Int ) { Confusing(R.string.notebookTypeConfusing, R.drawable.help, R.color.icon_error), - Important(R.string.notebookTypeImportant, R.drawable.flag_2, R.color.icon_action), + Important(R.string.notebookTypeImportant, R.drawable.keep_pin, R.color.icon_action), } diff --git a/libs/horizon/src/main/java/com/instructure/horizon/horizonui/foundation/HorizonColors.kt b/libs/horizon/src/main/java/com/instructure/horizon/horizonui/foundation/HorizonColors.kt index af8f653b7c..f4b25150a2 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/horizonui/foundation/HorizonColors.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/horizonui/foundation/HorizonColors.kt @@ -245,6 +245,8 @@ object HorizonColors { } object PrimitivesSea { + @Composable + fun sea12() = colorResource(R.color.primitives_sea12) @Composable fun sea30() = colorResource(R.color.primitives_sea30) @Composable diff --git a/libs/horizon/src/main/java/com/instructure/horizon/horizonui/molecules/DropdownChip.kt b/libs/horizon/src/main/java/com/instructure/horizon/horizonui/molecules/DropdownChip.kt new file mode 100644 index 0000000000..7f33063046 --- /dev/null +++ b/libs/horizon/src/main/java/com/instructure/horizon/horizonui/molecules/DropdownChip.kt @@ -0,0 +1,193 @@ +/* + * Copyright (C) 2025 - present Instructure, Inc. + * + * 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 com.instructure.horizon.horizonui.molecules + +import androidx.annotation.DrawableRes +import androidx.compose.foundation.BorderStroke +import androidx.compose.foundation.background +import androidx.compose.foundation.border +import androidx.compose.foundation.clickable +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.PaddingValues +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.size +import androidx.compose.foundation.layout.wrapContentSize +import androidx.compose.foundation.shape.RoundedCornerShape +import androidx.compose.material3.DropdownMenu +import androidx.compose.material3.DropdownMenuItem +import androidx.compose.material3.Icon +import androidx.compose.material3.MenuDefaults +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.graphics.Color +import androidx.compose.ui.res.painterResource +import androidx.compose.ui.tooling.preview.Preview +import androidx.compose.ui.unit.DpOffset +import androidx.compose.ui.unit.dp +import androidx.compose.ui.window.PopupProperties +import com.instructure.horizon.R +import com.instructure.horizon.horizonui.foundation.HorizonBorder +import com.instructure.horizon.horizonui.foundation.HorizonColors +import com.instructure.horizon.horizonui.foundation.HorizonCornerRadius +import com.instructure.horizon.horizonui.foundation.HorizonTypography + +data class DropdownItem( + val value: T, + val label: String, + @DrawableRes val iconRes: Int? = null, + val iconTint: Color? = null, + val backgroundColor: Color? = null +) + +@Composable +fun DropdownChip( + items: List>, + selectedItem: DropdownItem?, + onItemSelected: (DropdownItem?) -> Unit, + placeholder: String, + modifier: Modifier = Modifier, + borderColor: Color = HorizonColors.LineAndBorder.containerStroke(), + contentColor: Color = HorizonColors.Text.body() +) { + var expanded by remember { mutableStateOf(true) } + + Box(modifier = modifier) { + Row( + verticalAlignment = Alignment.CenterVertically, + modifier = Modifier + .border( + HorizonBorder.level1(color = borderColor), + HorizonCornerRadius.level1 + ) + .clickable { expanded = true } + .padding(horizontal = 8.dp, vertical = 2.dp) + ) { + Text( + text = selectedItem?.label ?: placeholder, + style = HorizonTypography.p2, + color = contentColor, + modifier = Modifier.padding(end = 2.dp) + ) + + Icon( + painter = painterResource(R.drawable.keyboard_arrow_down), + contentDescription = null, + modifier = Modifier.size(16.dp), + tint = contentColor + ) + } + + DropdownMenu( + expanded = expanded, + onDismissRequest = { expanded = false }, + shape = RoundedCornerShape(16.dp), + border = BorderStroke(1.dp, HorizonColors.LineAndBorder.containerStroke()), + containerColor = HorizonColors.Surface.cardPrimary(), + offset = DpOffset(0.dp, 4.dp), + modifier = Modifier.wrapContentSize() + ) { + items.forEach { item -> + val isSelected = selectedItem?.value == item.value + DropdownMenuItem( + text = { + Row( + verticalAlignment = Alignment.CenterVertically, + modifier = Modifier + .then( + if (/*isSelected*/ true && item.backgroundColor != null) { + Modifier.background( + item.backgroundColor + ) + } else { + Modifier + } + ) + .fillMaxSize() + .padding(horizontal = 8.dp) + ) { + item.iconRes?.let { iconRes -> + Icon( + painter = painterResource(iconRes), + contentDescription = null, + modifier = Modifier + .size(20.dp) + .padding(end = 4.dp), + tint = item.iconTint ?: HorizonColors.Icon.default() + ) + } + Text( + text = item.label, + style = HorizonTypography.p2, + color = if (isSelected && item.backgroundColor != null) { + item.iconTint ?: HorizonColors.Text.body() + } else { + HorizonColors.Text.body() + } + ) + } + }, + onClick = { + onItemSelected(item) + expanded = false + }, + colors = MenuDefaults.itemColors( + textColor = HorizonColors.Text.body() + ), + contentPadding = PaddingValues(0.dp), + modifier = Modifier.background(item.backgroundColor ?: Color.Transparent) + ) + } + } + } +} + +@Composable +@Preview +private fun DropdownChipPreview() { + val items = listOf( + DropdownItem( + value = "important", + label = "Important", + iconRes = R.drawable.flag_2, + iconTint = HorizonColors.Icon.action(), + backgroundColor = Color.Red + ), + DropdownItem( + value = "unclear", + label = "Unclear", + iconRes = R.drawable.help, + iconTint = HorizonColors.Icon.error() + ) + ) + + var selectedItem by remember { mutableStateOf?>(items[0]) } + + DropdownChip( + items = items, + selectedItem = selectedItem, + onItemSelected = { selectedItem = it }, + placeholder = "Type" + ) +} diff --git a/libs/horizon/src/main/res/values/colors.xml b/libs/horizon/src/main/res/values/colors.xml index 0eda1a2aa0..2dc60c0244 100644 --- a/libs/horizon/src/main/res/values/colors.xml +++ b/libs/horizon/src/main/res/values/colors.xml @@ -125,6 +125,7 @@ #036549 #02533C #024531 + #DAEEEF #37A1AA #1E95A0 #0A8C97 diff --git a/libs/horizon/src/main/res/values/strings.xml b/libs/horizon/src/main/res/values/strings.xml index 3e1308d2e4..78d78379bd 100644 --- a/libs/horizon/src/main/res/values/strings.xml +++ b/libs/horizon/src/main/res/values/strings.xml @@ -247,10 +247,13 @@ Once you submit this attempt, you won’t be able to make any changes. Cancel Submit attempt - Confusing + Unclear Important + All notes Notebook Filter + All notes + All courses Notes This is where all your notes, taken directly within your learning objects, are stored and organized. It\'s your personal hub for keeping track of key insights, important excerpts, and reflections as you learn. Dive in to review or expand on your notes anytime! Highlight From 8b9256e57e23b3abee8bdca91890adfb82e46194 Mon Sep 17 00:00:00 2001 From: "andras.maczak" Date: Fri, 7 Nov 2025 16:13:29 +0100 Subject: [PATCH 02/33] finished DropdownChip, modifications on NotebookScreen --- .../features/notebook/NotebookScreen.kt | 21 +- .../horizonui/molecules/DropdownChip.kt | 198 +++++++++++------- 2 files changed, 131 insertions(+), 88 deletions(-) diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt index ef32291e32..67b35d9d6c 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt @@ -63,8 +63,6 @@ import com.instructure.horizon.horizonui.foundation.SpaceSize import com.instructure.horizon.horizonui.foundation.horizonShadow import com.instructure.horizon.horizonui.molecules.DropdownChip import com.instructure.horizon.horizonui.molecules.DropdownItem -import com.instructure.horizon.horizonui.molecules.FilterDropdownChip -import com.instructure.horizon.horizonui.molecules.FilterDropdownItem import com.instructure.horizon.horizonui.molecules.IconButton import com.instructure.horizon.horizonui.molecules.IconButtonColor import com.instructure.horizon.horizonui.molecules.IconButtonSize @@ -185,10 +183,11 @@ private fun FilterContent( value = null as NotebookType?, label = context.getString(R.string.notebookTypeAllNotes), iconRes = R.drawable.menu, - iconTint = HorizonColors.Icon.default() + iconTint = HorizonColors.Icon.default(), + backgroundColor = HorizonColors.PrimitivesGrey.grey12() ) - val importantColor = HorizonColors.PrimitivesSea.sea12() - val confusingColor = HorizonColors.PrimitivesRed.red12() + val importantBgColor = HorizonColors.PrimitivesSea.sea12() + val confusingBgColor = HorizonColors.PrimitivesRed.red12() val typeItems = remember { @@ -198,15 +197,15 @@ private fun FilterContent( value = NotebookType.Important, label = context.getString(NotebookType.Important.labelRes), iconRes = NotebookType.Important.iconRes, - iconTint = context.getColor(NotebookType.Important.color).let { Color(it) }, - backgroundColor = importantColor + iconTint = Color(context.getColor(NotebookType.Important.color)), + backgroundColor = importantBgColor ), DropdownItem( value = NotebookType.Confusing, label = context.getString(NotebookType.Confusing.labelRes), iconRes = NotebookType.Confusing.iconRes, - iconTint = context.getColor(NotebookType.Confusing.color).let { androidx.compose.ui.graphics.Color(it) }, - backgroundColor = confusingColor + iconTint = Color(context.getColor(NotebookType.Confusing.color)), + backgroundColor = confusingBgColor ) ) } @@ -231,7 +230,9 @@ private fun FilterContent( selectedItem = selectedTypeItem, onItemSelected = { item -> onFilterSelected(item?.value) }, placeholder = stringResource(R.string.notebookFilterTypePlaceholder), - modifier = Modifier.weight(1f) + modifier = Modifier.weight(1f), + dropdownWidth = 178.dp, + verticalPadding = 6.dp ) } diff --git a/libs/horizon/src/main/java/com/instructure/horizon/horizonui/molecules/DropdownChip.kt b/libs/horizon/src/main/java/com/instructure/horizon/horizonui/molecules/DropdownChip.kt index 7f33063046..3e537fb35a 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/horizonui/molecules/DropdownChip.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/horizonui/molecules/DropdownChip.kt @@ -16,42 +16,47 @@ package com.instructure.horizon.horizonui.molecules import androidx.annotation.DrawableRes -import androidx.compose.foundation.BorderStroke +import androidx.compose.animation.core.animateIntAsState import androidx.compose.foundation.background import androidx.compose.foundation.border import androidx.compose.foundation.clickable -import androidx.compose.foundation.layout.Box -import androidx.compose.foundation.layout.PaddingValues +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.size -import androidx.compose.foundation.layout.wrapContentSize -import androidx.compose.foundation.shape.RoundedCornerShape -import androidx.compose.material3.DropdownMenu -import androidx.compose.material3.DropdownMenuItem import androidx.compose.material3.Icon -import androidx.compose.material3.MenuDefaults import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableIntStateOf 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.draw.clip +import androidx.compose.ui.draw.rotate import androidx.compose.ui.graphics.Color +import androidx.compose.ui.layout.onGloballyPositioned +import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.res.painterResource +import androidx.compose.ui.res.stringResource +import androidx.compose.ui.semantics.Role +import androidx.compose.ui.semantics.clearAndSetSemantics +import androidx.compose.ui.semantics.contentDescription +import androidx.compose.ui.semantics.role +import androidx.compose.ui.semantics.stateDescription import androidx.compose.ui.tooling.preview.Preview -import androidx.compose.ui.unit.DpOffset +import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp -import androidx.compose.ui.window.PopupProperties import com.instructure.horizon.R import com.instructure.horizon.horizonui.foundation.HorizonBorder import com.instructure.horizon.horizonui.foundation.HorizonColors import com.instructure.horizon.horizonui.foundation.HorizonCornerRadius import com.instructure.horizon.horizonui.foundation.HorizonTypography +import com.instructure.horizon.horizonui.organisms.inputs.common.InputDropDownPopup +import com.instructure.pandautils.compose.modifiers.conditional data class DropdownItem( val value: T, @@ -66,100 +71,135 @@ fun DropdownChip( items: List>, selectedItem: DropdownItem?, onItemSelected: (DropdownItem?) -> Unit, - placeholder: String, modifier: Modifier = Modifier, - borderColor: Color = HorizonColors.LineAndBorder.containerStroke(), - contentColor: Color = HorizonColors.Text.body() + dropdownWidth: Dp? = null, + placeholder: String, + borderColor: Color = HorizonColors.LineAndBorder.lineStroke(), + contentColor: Color = HorizonColors.Text.body(), + verticalPadding: Dp = 0.dp ) { - var expanded by remember { mutableStateOf(true) } + var isMenuOpen by remember { mutableStateOf(false) } + val localDensity = LocalDensity.current + var heightInPx by remember { mutableIntStateOf(0) } + var width by remember { mutableStateOf(dropdownWidth) } + val iconRotation = animateIntAsState( + targetValue = if (isMenuOpen) 180 else 0, + label = "iconRotation" + ) - Box(modifier = modifier) { + val expandedState = stringResource(R.string.a11y_expanded) + val collapsedState = stringResource(R.string.a11y_collapsed) + + Column(modifier = modifier) { Row( verticalAlignment = Alignment.CenterVertically, modifier = Modifier + .background( + if (isMenuOpen) { + HorizonColors.PrimitivesGrey.grey12() + } else { + HorizonColors.Surface.cardPrimary() + }, shape = HorizonCornerRadius.level1 + ) .border( HorizonBorder.level1(color = borderColor), HorizonCornerRadius.level1 ) - .clickable { expanded = true } + .clip(HorizonCornerRadius.level1) + .clickable { isMenuOpen = !isMenuOpen } .padding(horizontal = 8.dp, vertical = 2.dp) + .onGloballyPositioned { + heightInPx = it.size.height + if (dropdownWidth == null) { + width = with(localDensity) { it.size.width.toDp() } + } + } + .clearAndSetSemantics { + role = Role.DropdownList + stateDescription = if (isMenuOpen) expandedState else collapsedState + contentDescription = selectedItem?.label ?: placeholder + } ) { Text( text = selectedItem?.label ?: placeholder, style = HorizonTypography.p2, color = contentColor, - modifier = Modifier.padding(end = 2.dp) + modifier = Modifier.padding( + end = 2.dp, + top = verticalPadding, + bottom = verticalPadding + ) ) Icon( painter = painterResource(R.drawable.keyboard_arrow_down), contentDescription = null, - modifier = Modifier.size(16.dp), + modifier = Modifier + .size(16.dp) + .rotate(iconRotation.value.toFloat()), tint = contentColor ) } - DropdownMenu( - expanded = expanded, - onDismissRequest = { expanded = false }, - shape = RoundedCornerShape(16.dp), - border = BorderStroke(1.dp, HorizonColors.LineAndBorder.containerStroke()), - containerColor = HorizonColors.Surface.cardPrimary(), - offset = DpOffset(0.dp, 4.dp), - modifier = Modifier.wrapContentSize() - ) { - items.forEach { item -> - val isSelected = selectedItem?.value == item.value - DropdownMenuItem( - text = { - Row( - verticalAlignment = Alignment.CenterVertically, - modifier = Modifier - .then( - if (/*isSelected*/ true && item.backgroundColor != null) { - Modifier.background( - item.backgroundColor - ) - } else { - Modifier - } - ) - .fillMaxSize() - .padding(horizontal = 8.dp) - ) { - item.iconRes?.let { iconRes -> - Icon( - painter = painterResource(iconRes), - contentDescription = null, - modifier = Modifier - .size(20.dp) - .padding(end = 4.dp), - tint = item.iconTint ?: HorizonColors.Icon.default() - ) - } - Text( - text = item.label, - style = HorizonTypography.p2, - color = if (isSelected && item.backgroundColor != null) { - item.iconTint ?: HorizonColors.Text.body() - } else { - HorizonColors.Text.body() - } - ) - } - }, - onClick = { - onItemSelected(item) - expanded = false - }, - colors = MenuDefaults.itemColors( - textColor = HorizonColors.Text.body() - ), - contentPadding = PaddingValues(0.dp), - modifier = Modifier.background(item.backgroundColor ?: Color.Transparent) + InputDropDownPopup( + isMenuOpen = isMenuOpen, + options = items, + width = width, + verticalOffsetPx = heightInPx, + onMenuOpenChanged = { isMenuOpen = it }, + onOptionSelected = { item -> + if (selectedItem != item) { + onItemSelected(item) + } + }, + item = { item -> + DropdownChipItem(item, selectedItem) + } + ) + } +} + +@Composable +private fun DropdownChipItem( + item: DropdownItem, + selectedItem: DropdownItem? +) { + val isSelected = selectedItem?.value == item.value + Row( + verticalAlignment = Alignment.CenterVertically, + modifier = Modifier + .fillMaxWidth() + .conditional(isSelected && item.backgroundColor != null) { + background( + item.backgroundColor!!, ) } + .padding(horizontal = 12.dp, vertical = 10.dp) + ) { + item.iconRes?.let { iconRes -> + Icon( + painter = painterResource(iconRes), + contentDescription = null, + modifier = Modifier + .size(24.dp) + .padding(1.dp), + tint = if (isSelected) { + item.iconTint ?: HorizonColors.Icon.default() + } else { + HorizonColors.Icon.default() + } + ) } + Text( + text = item.label, + style = HorizonTypography.p1, + color = if (isSelected && item.backgroundColor != null) { + item.iconTint ?: HorizonColors.Text.body() + } else { + HorizonColors.Text.body() + }, + modifier = Modifier.padding(start = 4.dp) + ) } } @@ -172,7 +212,7 @@ private fun DropdownChipPreview() { label = "Important", iconRes = R.drawable.flag_2, iconTint = HorizonColors.Icon.action(), - backgroundColor = Color.Red + backgroundColor = HorizonColors.PrimitivesSea.sea12() ), DropdownItem( value = "unclear", @@ -188,6 +228,8 @@ private fun DropdownChipPreview() { items = items, selectedItem = selectedItem, onItemSelected = { selectedItem = it }, + dropdownWidth = 140.dp, + verticalPadding = 6.dp, placeholder = "Type" ) } From cf003d08e019420b41b4ae59951f4491d062ab83 Mon Sep 17 00:00:00 2001 From: "andras.maczak" Date: Mon, 10 Nov 2025 11:52:33 +0100 Subject: [PATCH 03/33] infinite scrolling --- .../features/notebook/NotebookScreen.kt | 88 ++++++++----------- .../features/notebook/NotebookUiState.kt | 3 +- .../features/notebook/NotebookViewModel.kt | 28 +++--- 3 files changed, 52 insertions(+), 67 deletions(-) diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt index 67b35d9d6c..ddd8022585 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt @@ -32,6 +32,8 @@ import androidx.compose.foundation.lazy.rememberLazyListState import androidx.compose.material3.Scaffold import androidx.compose.material3.Text import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.derivedStateOf import androidx.compose.runtime.remember import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier @@ -63,9 +65,6 @@ import com.instructure.horizon.horizonui.foundation.SpaceSize import com.instructure.horizon.horizonui.foundation.horizonShadow import com.instructure.horizon.horizonui.molecules.DropdownChip import com.instructure.horizon.horizonui.molecules.DropdownItem -import com.instructure.horizon.horizonui.molecules.IconButton -import com.instructure.horizon.horizonui.molecules.IconButtonColor -import com.instructure.horizon.horizonui.molecules.IconButtonSize import com.instructure.horizon.horizonui.molecules.Spinner import com.instructure.horizon.navigation.MainNavigationRoute import com.instructure.pandautils.compose.modifiers.conditional @@ -153,21 +152,33 @@ fun NotebookScreen( } } - item { - Column { - HorizonSpace(SpaceSize.SPACE_24) - - NotesPager( - canNavigateBack = state.hasPreviousPage, - canNavigateForward = state.hasNextPage, - isLoading = state.isLoading, - onNavigateBack = state.loadPreviousPage, - onNavigateForward = state.loadNextPage - ) + if (state.isLoadingMore) { + item { + LoadingContent() } } } } + + // Infinite scroll: Load more when scrolled near bottom + val shouldLoadMore = remember(state) { + derivedStateOf { + val lastVisibleItem = scrollState.layoutInfo.visibleItemsInfo.lastOrNull() + val totalItems = scrollState.layoutInfo.totalItemsCount + lastVisibleItem != null && + lastVisibleItem.index >= totalItems - 1 && + state.hasNextPage && + !state.isLoadingMore && + !state.isLoading && + state.notes.isNotEmpty() + } + } + + LaunchedEffect(shouldLoadMore.value) { + if (shouldLoadMore.value) { + state.loadNextPage() + } + } } } } @@ -210,7 +221,8 @@ private fun FilterContent( ) } - val selectedTypeItem = if (selectedFilter == null) allNotesItem else typeItems.find { it.value == selectedFilter } + val selectedTypeItem = + if (selectedFilter == null) allNotesItem else typeItems.find { it.value == selectedFilter } Column { Row( @@ -335,40 +347,6 @@ private fun EmptyContent(modifier: Modifier = Modifier) { } } -@Composable -private fun NotesPager( - canNavigateBack: Boolean, - canNavigateForward: Boolean, - isLoading: Boolean, - onNavigateBack: () -> Unit, - onNavigateForward: () -> Unit, -) { - if (canNavigateBack || canNavigateForward) { - Row( - horizontalArrangement = Arrangement.Center, - modifier = Modifier.fillMaxWidth() - ) { - IconButton( - iconRes = R.drawable.chevron_left, - color = IconButtonColor.Black, - size = IconButtonSize.SMALL, - onClick = onNavigateBack, - enabled = canNavigateBack && !isLoading - ) - - HorizonSpace(SpaceSize.SPACE_8) - - IconButton( - iconRes = R.drawable.chevron_right, - color = IconButtonColor.Black, - size = IconButtonSize.SMALL, - onClick = onNavigateForward, - enabled = canNavigateForward && !isLoading - ) - } - } -} - @Composable @Preview private fun NotebookScreenPreview() { @@ -385,7 +363,11 @@ private fun NotebookScreenPreview() { objectId = "456", objectType = NoteObjectType.PAGE, userText = "This is a note about an assignment.", - highlightedText = NoteHighlightedData("Important part of the assignment.", NoteHighlightedDataRange(0, 0, "", ""), NoteHighlightedDataTextPosition(0, 0)), + highlightedText = NoteHighlightedData( + "Important part of the assignment.", + NoteHighlightedDataRange(0, 0, "", ""), + NoteHighlightedDataTextPosition(0, 0) + ), updatedAt = Date(), type = NotebookType.Important ), @@ -395,7 +377,11 @@ private fun NotebookScreenPreview() { objectId = "789", objectType = NoteObjectType.PAGE, userText = "This is a note about another assignment.", - highlightedText = NoteHighlightedData("Confusing part of the assignment.", NoteHighlightedDataRange(0, 0, "", ""), NoteHighlightedDataTextPosition(0, 0)), + highlightedText = NoteHighlightedData( + "Confusing part of the assignment.", + NoteHighlightedDataRange(0, 0, "", ""), + NoteHighlightedDataTextPosition(0, 0) + ), updatedAt = Date(), type = NotebookType.Confusing ) diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookUiState.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookUiState.kt index 2fb4184155..24c90bb79c 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookUiState.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookUiState.kt @@ -21,12 +21,11 @@ import com.instructure.horizon.features.notebook.common.model.NotebookType data class NotebookUiState( val isLoading: Boolean = true, + val isLoadingMore: Boolean = false, val selectedFilter: NotebookType? = null, val onFilterSelected: (NotebookType?) -> Unit = {}, val notes: List = emptyList(), - val hasPreviousPage: Boolean = false, val hasNextPage: Boolean = false, - val loadPreviousPage: () -> Unit = {}, val loadNextPage: () -> Unit = {}, val updateContent: (Long?, Pair?) -> Unit, val showTopBar: Boolean = false, diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookViewModel.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookViewModel.kt index 262e6aacd5..b5a0390c2f 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookViewModel.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookViewModel.kt @@ -41,7 +41,6 @@ class NotebookViewModel @Inject constructor( private var objectTypeAndId: Pair? = null private val _uiState = MutableStateFlow(NotebookUiState( - loadPreviousPage = ::getPreviousPage, loadNextPage = ::getNextPage, onFilterSelected = ::onFilterSelected, updateContent = ::updateContent @@ -55,17 +54,21 @@ class NotebookViewModel @Inject constructor( private fun loadData( after: String? = null, - before: String? = null, courseId: Long? = this.courseId, + isLoadingMore: Boolean = false ) { viewModelScope.tryLaunch { _uiState.update { - it.copy(isLoading = true) + if (isLoadingMore) { + it.copy(isLoadingMore = true) + } else { + it.copy(isLoading = true) + } } val notesResponse = repository.getNotes( after = after, - before = before, + before = null, filterType = uiState.value.selectedFilter, courseId = courseId, objectTypeAndId = objectTypeAndId, @@ -74,13 +77,13 @@ class NotebookViewModel @Inject constructor( cursorId = notesResponse.edges?.firstOrNull()?.cursor pageInfo = notesResponse.pageInfo - val notes = notesResponse.mapToNotes() + val newNotes = notesResponse.mapToNotes() _uiState.update { it.copy( isLoading = false, - notes = notes, - hasPreviousPage = notesResponse.pageInfo.hasPreviousPage, + isLoadingMore = false, + notes = if (isLoadingMore) it.notes + newNotes else newNotes, hasNextPage = notesResponse.pageInfo.hasNextPage, ) } @@ -88,8 +91,7 @@ class NotebookViewModel @Inject constructor( _uiState.update { it.copy( isLoading = false, - notes = emptyList(), - hasPreviousPage = false, + isLoadingMore = false, hasNextPage = false, ) } @@ -97,11 +99,9 @@ class NotebookViewModel @Inject constructor( } private fun getNextPage() { - loadData(after = pageInfo?.endCursor) - } - - private fun getPreviousPage() { - loadData(before = pageInfo?.startCursor) + if (!uiState.value.isLoadingMore && uiState.value.hasNextPage) { + loadData(after = pageInfo?.endCursor, isLoadingMore = true) + } } private fun onFilterSelected(newFilter: NotebookType?) { From 9051fe2cb947cec300abebd6a245227a42b88890 Mon Sep 17 00:00:00 2001 From: "andras.maczak" Date: Tue, 11 Nov 2025 21:10:51 +0100 Subject: [PATCH 04/33] course filtering implemented, ui changes --- .../features/notebook/NotebookRepository.kt | 13 ++ .../features/notebook/NotebookScreen.kt | 136 ++++++++++++------ .../features/notebook/NotebookUiState.kt | 7 +- .../features/notebook/NotebookViewModel.kt | 32 ++++- .../common/composable/NotebookAppBar.kt | 79 +++++++--- .../notebook/NotebookViewModelTest.kt | 6 +- 6 files changed, 201 insertions(+), 72 deletions(-) diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookRepository.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookRepository.kt index ecd66a204b..32e17571c7 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookRepository.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookRepository.kt @@ -17,7 +17,11 @@ package com.instructure.horizon.features.notebook import com.apollographql.apollo.api.Optional +import com.instructure.canvasapi2.managers.graphql.horizon.CourseWithProgress +import com.instructure.canvasapi2.managers.graphql.horizon.HorizonGetCoursesManager import com.instructure.canvasapi2.managers.graphql.horizon.redwood.RedwoodApiManager +import com.instructure.canvasapi2.utils.ApiPrefs +import com.instructure.canvasapi2.utils.DataResult import com.instructure.horizon.features.notebook.common.model.NotebookType import com.instructure.redwood.QueryNotesQuery import com.instructure.redwood.type.LearningObjectFilter @@ -28,6 +32,8 @@ import javax.inject.Inject class NotebookRepository @Inject constructor( private val redwoodApiManager: RedwoodApiManager, + private val horizonGetCoursesManager: HorizonGetCoursesManager, + private val apiPrefs: ApiPrefs, ) { suspend fun getNotes( after: String? = null, @@ -79,4 +85,11 @@ class NotebookRepository @Inject constructor( } } + + suspend fun getCourses(forceNetwork: Boolean = false): DataResult> { + return horizonGetCoursesManager.getCoursesWithProgress( + userId = apiPrefs.user?.id ?: 0L, + forceNetwork = forceNetwork + ) + } } \ No newline at end of file diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt index ddd8022585..0f80c044d3 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt @@ -23,6 +23,7 @@ import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.PaddingValues import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.fillMaxHeight import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding @@ -45,6 +46,7 @@ import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.unit.dp import androidx.navigation.NavHostController +import com.instructure.canvasapi2.managers.graphql.horizon.CourseWithProgress import com.instructure.canvasapi2.managers.graphql.horizon.redwood.NoteHighlightedData import com.instructure.canvasapi2.managers.graphql.horizon.redwood.NoteHighlightedDataRange import com.instructure.canvasapi2.managers.graphql.horizon.redwood.NoteHighlightedDataTextPosition @@ -65,9 +67,9 @@ import com.instructure.horizon.horizonui.foundation.SpaceSize import com.instructure.horizon.horizonui.foundation.horizonShadow import com.instructure.horizon.horizonui.molecules.DropdownChip import com.instructure.horizon.horizonui.molecules.DropdownItem +import com.instructure.horizon.horizonui.molecules.HorizonDivider import com.instructure.horizon.horizonui.molecules.Spinner import com.instructure.horizon.navigation.MainNavigationRoute -import com.instructure.pandautils.compose.modifiers.conditional import com.instructure.pandautils.utils.localisedFormat import java.util.Date @@ -85,45 +87,54 @@ fun NotebookScreen( if (state.showTopBar) { NotebookAppBar( navigateBack = { mainNavController.popBackStack() }, - modifier = Modifier.conditional(scrollState.canScrollBackward) { - horizonShadow( - elevation = HorizonElevation.level2, - ) - } + centeredTitle = true ) } else if (onDismiss != null) { NotebookAppBar( onClose = { onDismiss() }, - modifier = Modifier.conditional(scrollState.canScrollBackward) { - horizonShadow( - elevation = HorizonElevation.level2, - ) - } + containerColor = HorizonColors.Surface.pageSecondary() ) } }, ) { padding -> - Box( - contentAlignment = Alignment.TopCenter, + Column( + horizontalAlignment = Alignment.CenterHorizontally, modifier = Modifier .fillMaxSize() .padding(paddingValues = padding) - .clip(HorizonCornerRadius.level5) - .background(HorizonColors.Surface.pageSecondary()) ) { + if ((state.showNoteTypeFilter || state.showCourseFilter) && state.notes.isNotEmpty()) { + Box { + FilterContent( + modifier = Modifier + .clip(HorizonCornerRadius.level5) + .background(HorizonColors.Surface.pageSecondary()), + selectedFilter = state.selectedFilter, + onFilterSelected = state.onFilterSelected, + selectedCourse = state.selectedCourse, + onCourseSelected = state.onCourseSelected, + courses = state.courses, + showNoteTypeFilter = state.showNoteTypeFilter, + showCourseTypeFilter = state.showCourseFilter + ) + } + if (scrollState.canScrollBackward) { + HorizonDivider() + } + } + LazyColumn( state = scrollState, - modifier = Modifier, - contentPadding = PaddingValues(horizontal = 16.dp, vertical = 24.dp) + modifier = Modifier + .fillMaxHeight() + .background(HorizonColors.Surface.pageSecondary()), + contentPadding = PaddingValues( + start = 16.dp, + end = 16.dp, + top = 16.dp, + bottom = 16.dp + ) ) { - if (state.showFilters && state.notes.isNotEmpty()) { - item { - FilterContent( - state.selectedFilter, - state.onFilterSelected - ) - } - } if (state.isLoading) { item { @@ -166,11 +177,11 @@ fun NotebookScreen( val lastVisibleItem = scrollState.layoutInfo.visibleItemsInfo.lastOrNull() val totalItems = scrollState.layoutInfo.totalItemsCount lastVisibleItem != null && - lastVisibleItem.index >= totalItems - 1 && - state.hasNextPage && - !state.isLoadingMore && - !state.isLoading && - state.notes.isNotEmpty() + lastVisibleItem.index >= totalItems - 1 && + state.hasNextPage && + !state.isLoadingMore && + !state.isLoading && + state.notes.isNotEmpty() } } @@ -185,11 +196,18 @@ fun NotebookScreen( @Composable private fun FilterContent( + modifier: Modifier = Modifier, selectedFilter: NotebookType?, onFilterSelected: (NotebookType?) -> Unit, + selectedCourse: CourseWithProgress?, + onCourseSelected: (CourseWithProgress?) -> Unit, + courses: List, + showNoteTypeFilter: Boolean = true, + showCourseTypeFilter: Boolean = true, ) { val context = LocalContext.current + // Type filter items val allNotesItem = DropdownItem( value = null as NotebookType?, label = context.getString(R.string.notebookTypeAllNotes), @@ -201,7 +219,6 @@ private fun FilterContent( val confusingBgColor = HorizonColors.PrimitivesRed.red12() val typeItems = remember { - listOf( allNotesItem, DropdownItem( @@ -221,34 +238,60 @@ private fun FilterContent( ) } + // Course filter items + val allCoursesItem = DropdownItem( + value = null as CourseWithProgress?, + label = context.getString(R.string.notebookFilterCoursePlaceholder), + iconRes = null, + iconTint = null, + backgroundColor = HorizonColors.PrimitivesGrey.grey12() + ) + + val courseItems = remember(courses) { + listOf(allCoursesItem) + courses.map { course -> + DropdownItem( + value = course, + label = course.courseName, + iconRes = null, + iconTint = null + ) + } + } + val selectedTypeItem = if (selectedFilter == null) allNotesItem else typeItems.find { it.value == selectedFilter } - Column { - Row( - modifier = Modifier.fillMaxWidth(), - horizontalArrangement = Arrangement.spacedBy(8.dp) - ) { - /*FilterDropdownChip( - items = emptyList(), - selectedItem = null, - onItemSelected = { }, + val selectedCourseItem = + if (selectedCourse == null) allCoursesItem else courseItems.find { it.value == selectedCourse } + + + Row( + modifier = modifier + .fillMaxWidth() + .padding(16.dp), + horizontalArrangement = Arrangement.spacedBy(8.dp) + ) { + if (showCourseTypeFilter) { + DropdownChip( + items = courseItems, + selectedItem = selectedCourseItem, + onItemSelected = { item -> onCourseSelected(item?.value) }, placeholder = stringResource(R.string.notebookFilterCoursePlaceholder), - modifier = Modifier.weight(1f) - )*/ + dropdownWidth = 178.dp, + verticalPadding = 6.dp + ) + } + if (showNoteTypeFilter) { DropdownChip( items = typeItems, selectedItem = selectedTypeItem, onItemSelected = { item -> onFilterSelected(item?.value) }, placeholder = stringResource(R.string.notebookFilterTypePlaceholder), - modifier = Modifier.weight(1f), dropdownWidth = 178.dp, verticalPadding = 6.dp ) } - - HorizonSpace(SpaceSize.SPACE_16) } } @@ -353,7 +396,8 @@ private fun NotebookScreenPreview() { ContextKeeper.appContext = LocalContext.current val state = NotebookUiState( isLoading = false, - showFilters = true, + showNoteTypeFilter = true, + showCourseFilter = true, showTopBar = true, selectedFilter = NotebookType.Important, notes = listOf( diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookUiState.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookUiState.kt index 24c90bb79c..de7ef8ae19 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookUiState.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookUiState.kt @@ -16,6 +16,7 @@ */ package com.instructure.horizon.features.notebook +import com.instructure.canvasapi2.managers.graphql.horizon.CourseWithProgress import com.instructure.horizon.features.notebook.common.model.Note import com.instructure.horizon.features.notebook.common.model.NotebookType @@ -24,10 +25,14 @@ data class NotebookUiState( val isLoadingMore: Boolean = false, val selectedFilter: NotebookType? = null, val onFilterSelected: (NotebookType?) -> Unit = {}, + val selectedCourse: CourseWithProgress? = null, + val onCourseSelected: (CourseWithProgress?) -> Unit = {}, + val courses: List = emptyList(), val notes: List = emptyList(), val hasNextPage: Boolean = false, val loadNextPage: () -> Unit = {}, val updateContent: (Long?, Pair?) -> Unit, val showTopBar: Boolean = false, - val showFilters: Boolean = false, + val showNoteTypeFilter: Boolean = false, + val showCourseFilter: Boolean = false, ) \ No newline at end of file diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookViewModel.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookViewModel.kt index b5a0390c2f..a7064772c4 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookViewModel.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookViewModel.kt @@ -18,6 +18,8 @@ package com.instructure.horizon.features.notebook import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope +import com.instructure.canvasapi2.managers.graphql.horizon.CourseWithProgress +import com.instructure.canvasapi2.utils.DataResult import com.instructure.canvasapi2.utils.weave.catch import com.instructure.canvasapi2.utils.weave.tryLaunch import com.instructure.horizon.features.notebook.common.model.NotebookType @@ -28,6 +30,7 @@ import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.update +import kotlinx.coroutines.launch import javax.inject.Inject @HiltViewModel @@ -43,15 +46,30 @@ class NotebookViewModel @Inject constructor( private val _uiState = MutableStateFlow(NotebookUiState( loadNextPage = ::getNextPage, onFilterSelected = ::onFilterSelected, + onCourseSelected = ::onCourseSelected, updateContent = ::updateContent )) val uiState = _uiState.asStateFlow() init { + loadCourses() loadData() updateScreenState() } + private fun loadCourses() { + viewModelScope.launch { + when (val result = repository.getCourses()) { + is DataResult.Success -> { + _uiState.update { it.copy(courses = result.data) } + } + is DataResult.Fail -> { + _uiState.update { it.copy(courses = emptyList()) } + } + } + } + } + private fun loadData( after: String? = null, courseId: Long? = this.courseId, @@ -111,6 +129,14 @@ class NotebookViewModel @Inject constructor( loadData() } + private fun onCourseSelected(course: CourseWithProgress?) { + _uiState.update { currentState -> + currentState.copy(selectedCourse = course) + } + courseId = course?.courseId + loadData(courseId = courseId) + } + private fun updateContent(courseId: Long?, objectTypeAndId: Pair?) { if (courseId != this.courseId || objectTypeAndId != this.objectTypeAndId) { this.courseId = courseId @@ -132,12 +158,12 @@ class NotebookViewModel @Inject constructor( if (courseId != null) { _uiState.update { it.copy(showTopBar = false) } if (objectTypeAndId != null) { - _uiState.update { it.copy(showFilters = false) } + _uiState.update { it.copy(showNoteTypeFilter = false, showCourseFilter = false) } } else { - _uiState.update { it.copy(showFilters = true) } + _uiState.update { it.copy(showNoteTypeFilter = true, showCourseFilter = false) } } } else { - _uiState.update { it.copy(showTopBar = true, showFilters = true) } + _uiState.update { it.copy(showTopBar = true, showNoteTypeFilter = true, showCourseFilter = true) } } } } \ No newline at end of file diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/common/composable/NotebookAppBar.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/common/composable/NotebookAppBar.kt index 6b3284077e..3243e82ea9 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/common/composable/NotebookAppBar.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/common/composable/NotebookAppBar.kt @@ -17,16 +17,18 @@ package com.instructure.horizon.features.notebook.common.composable import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size +import androidx.compose.material3.CenterAlignedTopAppBar import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.Icon import androidx.compose.material3.Text -import androidx.compose.material3.TopAppBar import androidx.compose.material3.TopAppBarDefaults import androidx.compose.runtime.Composable import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.graphics.Color import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.res.painterResource import androidx.compose.ui.res.stringResource @@ -46,26 +48,37 @@ import com.instructure.horizon.horizonui.molecules.IconButtonSize fun NotebookAppBar( modifier: Modifier = Modifier, navigateBack: (() -> Unit)? = null, - onClose: (() -> Unit)? = null + onClose: (() -> Unit)? = null, + centeredTitle: Boolean = false, + containerColor: Color = HorizonColors.Surface.pagePrimary(), ) { - TopAppBar( + CenterAlignedTopAppBar( title = { - Row( - verticalAlignment = Alignment.CenterVertically - ) { - Icon( - painter = painterResource(R.drawable.edit_note), - contentDescription = null, - tint = HorizonColors.Icon.default(), - modifier = Modifier - .size(20.dp) - .padding(end = 4.dp) - ) + if (centeredTitle) { Text( text = stringResource(R.string.notebookTitle), - style = HorizonTypography.h4, - color = HorizonColors.Text.title() + style = HorizonTypography.h3, + color = HorizonColors.Text.title(), ) + } else { + Row( + verticalAlignment = Alignment.CenterVertically, + modifier = Modifier.fillMaxWidth() + ) { + Icon( + painter = painterResource(R.drawable.edit_note), + contentDescription = null, + tint = HorizonColors.Icon.default(), + modifier = Modifier + .padding(start = 16.dp, end = 4.dp) + .size(20.dp) + ) + Text( + text = stringResource(R.string.notebookTitle), + style = HorizonTypography.h4, + color = HorizonColors.Text.title() + ) + } } }, navigationIcon = { @@ -73,9 +86,8 @@ fun NotebookAppBar( IconButton( iconRes = R.drawable.arrow_back, contentDescription = stringResource(R.string.a11yNavigateBack), - color = IconButtonColor.Inverse, + color = IconButtonColor.Ghost, size = IconButtonSize.SMALL, - elevation = HorizonElevation.level4, onClick = navigateBack, modifier = Modifier.padding(horizontal = 16.dp) ) @@ -95,7 +107,7 @@ fun NotebookAppBar( } }, colors = TopAppBarDefaults.topAppBarColors( - containerColor = HorizonColors.Surface.pagePrimary(), + containerColor = containerColor, titleContentColor = HorizonColors.Text.title(), navigationIconContentColor = HorizonColors.Icon.default() ), @@ -110,4 +122,33 @@ private fun NotebookAppBarPreview() { NotebookAppBar( navigateBack = {} ) +} + +@Composable +@Preview +private fun NotebookAppBarClosePreview() { + ContextKeeper.appContext = LocalContext.current + NotebookAppBar( + onClose = {} + ) +} + +@Composable +@Preview +private fun NotebookAppBarCenteredPreview() { + ContextKeeper.appContext = LocalContext.current + NotebookAppBar( + navigateBack = {}, + centeredTitle = true + ) +} + +@Composable +@Preview +private fun NotebookAppBarCenteredClosePreview() { + ContextKeeper.appContext = LocalContext.current + NotebookAppBar( + onClose = {}, + centeredTitle = true + ) } \ No newline at end of file diff --git a/libs/horizon/src/test/java/com/instructure/horizon/features/notebook/NotebookViewModelTest.kt b/libs/horizon/src/test/java/com/instructure/horizon/features/notebook/NotebookViewModelTest.kt index 2a2ab3ced0..4e98ba097c 100644 --- a/libs/horizon/src/test/java/com/instructure/horizon/features/notebook/NotebookViewModelTest.kt +++ b/libs/horizon/src/test/java/com/instructure/horizon/features/notebook/NotebookViewModelTest.kt @@ -173,7 +173,7 @@ class NotebookViewModelTest { viewModel.uiState.value.updateContent(123L, null) assertFalse(viewModel.uiState.value.showTopBar) - assertTrue(viewModel.uiState.value.showFilters) + assertTrue(viewModel.uiState.value.showNoteTypeFilter) } @Test @@ -183,7 +183,7 @@ class NotebookViewModelTest { viewModel.uiState.value.updateContent(123L, Pair("Assignment", "456")) assertFalse(viewModel.uiState.value.showTopBar) - assertFalse(viewModel.uiState.value.showFilters) + assertFalse(viewModel.uiState.value.showNoteTypeFilter) } @Test @@ -193,7 +193,7 @@ class NotebookViewModelTest { viewModel.uiState.value.updateContent(null, null) assertTrue(viewModel.uiState.value.showTopBar) - assertTrue(viewModel.uiState.value.showFilters) + assertTrue(viewModel.uiState.value.showNoteTypeFilter) } private fun getViewModel(): NotebookViewModel { From af1a5a53ed89a1ea28a2d4eb0f7cc8f1fc866213 Mon Sep 17 00:00:00 2001 From: "andras.maczak" Date: Wed, 12 Nov 2025 11:14:35 +0100 Subject: [PATCH 05/33] notebook ui changes --- .../features/notebook/NotebookScreen.kt | 154 +++++++++--------- libs/horizon/src/main/res/values/strings.xml | 1 + 2 files changed, 77 insertions(+), 78 deletions(-) diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt index 0f80c044d3..0d0a91a7ed 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt @@ -65,6 +65,10 @@ import com.instructure.horizon.horizonui.foundation.HorizonSpace import com.instructure.horizon.horizonui.foundation.HorizonTypography import com.instructure.horizon.horizonui.foundation.SpaceSize import com.instructure.horizon.horizonui.foundation.horizonShadow +import com.instructure.horizon.horizonui.molecules.Button +import com.instructure.horizon.horizonui.molecules.ButtonColor +import com.instructure.horizon.horizonui.molecules.ButtonHeight +import com.instructure.horizon.horizonui.molecules.ButtonWidth import com.instructure.horizon.horizonui.molecules.DropdownChip import com.instructure.horizon.horizonui.molecules.DropdownItem import com.instructure.horizon.horizonui.molecules.HorizonDivider @@ -104,90 +108,82 @@ fun NotebookScreen( .padding(paddingValues = padding) ) { if ((state.showNoteTypeFilter || state.showCourseFilter) && state.notes.isNotEmpty()) { - Box { - FilterContent( - modifier = Modifier - .clip(HorizonCornerRadius.level5) - .background(HorizonColors.Surface.pageSecondary()), - selectedFilter = state.selectedFilter, - onFilterSelected = state.onFilterSelected, - selectedCourse = state.selectedCourse, - onCourseSelected = state.onCourseSelected, - courses = state.courses, - showNoteTypeFilter = state.showNoteTypeFilter, - showCourseTypeFilter = state.showCourseFilter - ) - } - if (scrollState.canScrollBackward) { - HorizonDivider() - } + FilterContent( + modifier = Modifier + .clip(HorizonCornerRadius.level5) + .background(HorizonColors.Surface.pageSecondary()), + selectedFilter = state.selectedFilter, + onFilterSelected = state.onFilterSelected, + selectedCourse = state.selectedCourse, + onCourseSelected = state.onCourseSelected, + courses = state.courses, + showNoteTypeFilter = state.showNoteTypeFilter, + showCourseTypeFilter = state.showCourseFilter + ) } - LazyColumn( - state = scrollState, - modifier = Modifier - .fillMaxHeight() - .background(HorizonColors.Surface.pageSecondary()), - contentPadding = PaddingValues( - start = 16.dp, - end = 16.dp, - top = 16.dp, - bottom = 16.dp - ) - ) { + Box { + LazyColumn( + state = scrollState, + modifier = Modifier + .fillMaxHeight() + .background(HorizonColors.Surface.pageSecondary()), + contentPadding = PaddingValues( + start = 16.dp, + end = 16.dp, + top = 16.dp, + bottom = 16.dp + ) + ) { - if (state.isLoading) { - item { - LoadingContent() - } - } else if (state.notes.isEmpty()) { - item { - EmptyContent() - } - } else { - items(state.notes) { note -> - Column { - NoteContent(note) { - onNoteSelected?.invoke(note) ?: mainNavController.navigate( - MainNavigationRoute.ModuleItemSequence( - courseId = note.courseId, - moduleItemAssetType = note.objectType.value, - moduleItemAssetId = note.objectId, + if (state.isLoading) { + item { + LoadingContent() + } + } else if (state.notes.isEmpty()) { + item { + EmptyContent() + } + } else { + items(state.notes) { note -> + Column { + NoteContent(note) { + onNoteSelected?.invoke(note) ?: mainNavController.navigate( + MainNavigationRoute.ModuleItemSequence( + courseId = note.courseId, + moduleItemAssetType = note.objectType.value, + moduleItemAssetId = note.objectId, + ) ) - ) - } + } - if (state.notes.lastOrNull() != note) { - HorizonSpace(SpaceSize.SPACE_4) + if (state.notes.lastOrNull() != note) { + HorizonSpace(SpaceSize.SPACE_4) + } } } - } - if (state.isLoadingMore) { - item { - LoadingContent() + if (state.hasNextPage) { + item { + if (state.isLoadingMore) { + LoadingContent() + } else { + Button( + label = stringResource(R.string.showMore), + height = ButtonHeight.SMALL, + width = ButtonWidth.FILL, + color = ButtonColor.WhiteWithOutline, + onClick = { state.loadNextPage() }, + modifier = Modifier.padding(vertical = 24.dp) + ) + } + } } } } - } - - // Infinite scroll: Load more when scrolled near bottom - val shouldLoadMore = remember(state) { - derivedStateOf { - val lastVisibleItem = scrollState.layoutInfo.visibleItemsInfo.lastOrNull() - val totalItems = scrollState.layoutInfo.totalItemsCount - lastVisibleItem != null && - lastVisibleItem.index >= totalItems - 1 && - state.hasNextPage && - !state.isLoadingMore && - !state.isLoading && - state.notes.isNotEmpty() - } - } - LaunchedEffect(shouldLoadMore.value) { - if (shouldLoadMore.value) { - state.loadNextPage() + if (scrollState.canScrollBackward) { + HorizonDivider() } } } @@ -206,6 +202,9 @@ private fun FilterContent( showCourseTypeFilter: Boolean = true, ) { val context = LocalContext.current + val defaultBackgroundColor = HorizonColors.PrimitivesGrey.grey12() + val importantBgColor = HorizonColors.PrimitivesSea.sea12() + val confusingBgColor = HorizonColors.PrimitivesRed.red12() // Type filter items val allNotesItem = DropdownItem( @@ -213,11 +212,9 @@ private fun FilterContent( label = context.getString(R.string.notebookTypeAllNotes), iconRes = R.drawable.menu, iconTint = HorizonColors.Icon.default(), - backgroundColor = HorizonColors.PrimitivesGrey.grey12() + backgroundColor = defaultBackgroundColor ) - val importantBgColor = HorizonColors.PrimitivesSea.sea12() - val confusingBgColor = HorizonColors.PrimitivesRed.red12() - + val typeItems = remember { listOf( allNotesItem, @@ -244,7 +241,7 @@ private fun FilterContent( label = context.getString(R.string.notebookFilterCoursePlaceholder), iconRes = null, iconTint = null, - backgroundColor = HorizonColors.PrimitivesGrey.grey12() + backgroundColor = defaultBackgroundColor ) val courseItems = remember(courses) { @@ -253,7 +250,8 @@ private fun FilterContent( value = course, label = course.courseName, iconRes = null, - iconTint = null + iconTint = null, + backgroundColor = defaultBackgroundColor ) } } diff --git a/libs/horizon/src/main/res/values/strings.xml b/libs/horizon/src/main/res/values/strings.xml index 78d78379bd..7baf1cd9ce 100644 --- a/libs/horizon/src/main/res/values/strings.xml +++ b/libs/horizon/src/main/res/values/strings.xml @@ -443,4 +443,5 @@ Next item Previous item %1$d of %2$d + Show more \ No newline at end of file From 32947afd78777ae95c6e1e9c8c22c7d73bb2a6cd Mon Sep 17 00:00:00 2001 From: "andras.maczak" Date: Thu, 13 Nov 2025 11:42:14 +0100 Subject: [PATCH 06/33] notebook ui changes --- .../learn/course/note/CourseNotesScreen.kt | 2 +- .../features/notebook/NotebookScreen.kt | 90 ++++++++++--------- .../features/notebook/NotebookUiState.kt | 4 +- .../features/notebook/NotebookViewModel.kt | 84 +++++++++++------ 4 files changed, 106 insertions(+), 74 deletions(-) diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/learn/course/note/CourseNotesScreen.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/learn/course/note/CourseNotesScreen.kt index 0f84853fe1..68ed097656 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/learn/course/note/CourseNotesScreen.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/learn/course/note/CourseNotesScreen.kt @@ -39,7 +39,7 @@ fun CourseNotesScreen( LaunchedEffect(courseId) { viewModel.updateFilters(courseId) - viewModel.updateScreenState(true, false) + viewModel.updateScreenState(showNoteTypeFilter = true, showCourseFilter = false, showTopBar = false) } Box( diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt index 452f53380f..528b90b48a 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt @@ -34,8 +34,6 @@ import androidx.compose.material3.Scaffold import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect -import androidx.compose.runtime.LaunchedEffect -import androidx.compose.runtime.derivedStateOf import androidx.compose.runtime.remember import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier @@ -77,7 +75,6 @@ import com.instructure.horizon.horizonui.molecules.DropdownItem import com.instructure.horizon.horizonui.molecules.HorizonDivider import com.instructure.horizon.horizonui.molecules.Spinner import com.instructure.horizon.navigation.MainNavigationRoute -import com.instructure.pandautils.compose.modifiers.conditional import com.instructure.pandautils.utils.ViewStyler import com.instructure.pandautils.utils.getActivityOrNull import com.instructure.pandautils.utils.localisedFormat @@ -90,7 +87,10 @@ fun NotebookScreen( ) { val activity = LocalContext.current.getActivityOrNull() LaunchedEffect(Unit) { - if (activity != null) ViewStyler.setStatusBarColor(activity, ContextCompat.getColor(activity, R.color.surface_pagePrimary)) + if (activity != null) ViewStyler.setStatusBarColor( + activity, + ContextCompat.getColor(activity, R.color.surface_pagePrimary) + ) } val scrollState = rememberLazyListState() @@ -111,7 +111,7 @@ fun NotebookScreen( .fillMaxSize() .padding(paddingValues = padding) ) { - if ((state.showNoteTypeFilter || state.showCourseFilter) && state.notes.isNotEmpty()) { + if ((state.showNoteTypeFilter || state.showCourseFilter) && (state.courses.isNotEmpty() || state.selectedCourse != null || state.selectedFilter != null)) { FilterContent( modifier = Modifier .clip(HorizonCornerRadius.level5) @@ -122,7 +122,7 @@ fun NotebookScreen( onCourseSelected = state.onCourseSelected, courses = state.courses, showNoteTypeFilter = state.showNoteTypeFilter, - showCourseTypeFilter = state.showCourseFilter + showCourseFilter = state.showCourseFilter ) } @@ -140,43 +140,47 @@ fun NotebookScreen( ) ) { - if (state.isLoading) { - item { - LoadingContent() - } - } else if (state.notes.isEmpty()) { - item { - EmptyContent() - } - } else { - items(state.notes) { note -> - Column { - NoteContent(note) { - if (state.navigateToEdit) { - mainNavController.navigate( - NotebookRoute.EditNotebook( - noteId = note.id, - highlightedTextStartOffset = note.highlightedText.range.startOffset, - highlightedTextEndOffset = note.highlightedText.range.endOffset, - highlightedTextStartContainer = note.highlightedText.range.startContainer, - highlightedTextEndContainer = note.highlightedText.range.endContainer, - textSelectionStart = note.highlightedText.textPosition.start, - textSelectionEnd = note.highlightedText.textPosition.end, - highlightedText = note.highlightedText.selectedText, - noteType = note.type.name, - userComment = note.userText - ) - ) - } else { - mainNavController.navigate( - MainNavigationRoute.ModuleItemSequence( - courseId = note.courseId, - moduleItemAssetType = note.objectType.value, - moduleItemAssetId = note.objectId, - ) - ) + if (state.isLoading) { + item { + LoadingContent() + } + } else if (state.notes.isEmpty()) { + if (state.selectedCourse != null || state.selectedFilter != null) { + + } else { + item { + EmptyContent() } } + } else { + items(state.notes) { note -> + Column { + NoteContent(note) { + if (state.navigateToEdit) { + mainNavController.navigate( + NotebookRoute.EditNotebook( + noteId = note.id, + highlightedTextStartOffset = note.highlightedText.range.startOffset, + highlightedTextEndOffset = note.highlightedText.range.endOffset, + highlightedTextStartContainer = note.highlightedText.range.startContainer, + highlightedTextEndContainer = note.highlightedText.range.endContainer, + textSelectionStart = note.highlightedText.textPosition.start, + textSelectionEnd = note.highlightedText.textPosition.end, + highlightedText = note.highlightedText.selectedText, + noteType = note.type.name, + userComment = note.userText + ) + ) + } else { + mainNavController.navigate( + MainNavigationRoute.ModuleItemSequence( + courseId = note.courseId, + moduleItemAssetType = note.objectType.value, + moduleItemAssetId = note.objectId, + ) + ) + } + } if (state.notes.lastOrNull() != note) { HorizonSpace(SpaceSize.SPACE_4) @@ -220,7 +224,7 @@ private fun FilterContent( onCourseSelected: (CourseWithProgress?) -> Unit, courses: List, showNoteTypeFilter: Boolean = true, - showCourseTypeFilter: Boolean = true, + showCourseFilter: Boolean = true, ) { val context = LocalContext.current val defaultBackgroundColor = HorizonColors.PrimitivesGrey.grey12() @@ -290,7 +294,7 @@ private fun FilterContent( .padding(16.dp), horizontalArrangement = Arrangement.spacedBy(8.dp) ) { - if (showCourseTypeFilter) { + if (showCourseFilter) { DropdownChip( items = courseItems, selectedItem = selectedCourseItem, diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookUiState.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookUiState.kt index 78406ee748..c15e544929 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookUiState.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookUiState.kt @@ -35,6 +35,6 @@ data class NotebookUiState( val showTopBar: Boolean = false, val showFilters: Boolean = false, val navigateToEdit: Boolean = false, - val showNoteTypeFilter: Boolean = false, - val showCourseFilter: Boolean = false, + val showNoteTypeFilter: Boolean = true, + val showCourseFilter: Boolean = true, ) \ No newline at end of file diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookViewModel.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookViewModel.kt index 688e10382a..fb4ce50d6c 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookViewModel.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookViewModel.kt @@ -29,6 +29,7 @@ import com.instructure.horizon.features.notebook.navigation.NotebookRoute import com.instructure.redwood.QueryNotesQuery import com.instructure.redwood.type.OrderDirection import dagger.hilt.android.lifecycle.HiltViewModel +import kotlinx.coroutines.Job import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.update @@ -39,28 +40,36 @@ import javax.inject.Inject class NotebookViewModel @Inject constructor( private val repository: NotebookRepository, savedStateHandle: SavedStateHandle, -): ViewModel() { +) : ViewModel() { private var cursorId: String? = null private var pageInfo: QueryNotesQuery.PageInfo? = null - private var courseId: Long? = savedStateHandle.get(NotebookRoute.Notebook.COURSE_ID)?.toLongOrNull() + private var courseId: Long? = + savedStateHandle.get(NotebookRoute.Notebook.COURSE_ID)?.toLongOrNull() private var objectTypeAndId: Pair? = getObjectTypeAndId(savedStateHandle) - private var showTopBar: Boolean = savedStateHandle.get(NotebookRoute.Notebook.SHOW_TOP_BAR) ?: false - private var showFilters: Boolean = savedStateHandle.get(NotebookRoute.Notebook.SHOW_FILTERS) ?: false - private var navigateToEdit: Boolean = savedStateHandle.get(NotebookRoute.Notebook.NAVIGATE_TO_EDIT) ?: false - - private val _uiState = MutableStateFlow(NotebookUiState( - loadNextPage = ::getNextPage, - onFilterSelected = ::onFilterSelected, - onCourseSelected = ::onCourseSelected, - updateContent = ::updateContent, - showTopBar = showTopBar, - showFilters = showFilters, - navigateToEdit = navigateToEdit, - )) + private var showTopBar: Boolean = + savedStateHandle.get(NotebookRoute.Notebook.SHOW_TOP_BAR) ?: false + private var showFilters: Boolean = + savedStateHandle.get(NotebookRoute.Notebook.SHOW_FILTERS) ?: false + private var navigateToEdit: Boolean = + savedStateHandle.get(NotebookRoute.Notebook.NAVIGATE_TO_EDIT) ?: false + + private val _uiState = MutableStateFlow( + NotebookUiState( + loadNextPage = ::getNextPage, + onFilterSelected = ::onFilterSelected, + onCourseSelected = ::onCourseSelected, + updateContent = ::updateContent, + showTopBar = showTopBar, + showFilters = showFilters, + navigateToEdit = navigateToEdit, + ) + ) val uiState = _uiState.asStateFlow() + var updateJob: Job? = null init { + removeCourseFilterIfNeeded() loadCourses() loadData() } @@ -71,6 +80,7 @@ class NotebookViewModel @Inject constructor( is DataResult.Success -> { _uiState.update { it.copy(courses = result.data) } } + is DataResult.Fail -> { _uiState.update { it.copy(courses = emptyList()) } } @@ -83,7 +93,7 @@ class NotebookViewModel @Inject constructor( courseId: Long? = this.courseId, isLoadingMore: Boolean = false ) { - viewModelScope.tryLaunch { + updateJob = viewModelScope.tryLaunch { _uiState.update { if (isLoadingMore) { it.copy(isLoadingMore = true) @@ -103,13 +113,25 @@ class NotebookViewModel @Inject constructor( cursorId = notesResponse.edges?.firstOrNull()?.cursor pageInfo = notesResponse.pageInfo - val newNotes = notesResponse.mapToNotes() + val oldNotes = if (courseId != null) { + uiState.value.notes.filter { it.courseId == this@NotebookViewModel.courseId } + } else { + uiState.value.notes + } + val newNotes = notesResponse.mapToNotes().also { notes -> + // Filter notes by courseId if applicable + if (courseId != null) { + notes.filter { it.courseId == courseId } + } else { + notes + } + } _uiState.update { it.copy( isLoading = false, isLoadingMore = false, - notes = if (isLoadingMore) it.notes + newNotes else newNotes, + notes = if (isLoadingMore) oldNotes + newNotes else newNotes, hasNextPage = notesResponse.pageInfo.hasNextPage, ) } @@ -134,6 +156,7 @@ class NotebookViewModel @Inject constructor( _uiState.update { currentState -> currentState.copy(selectedFilter = newFilter) } + updateJob?.cancel() loadData() } @@ -141,6 +164,7 @@ class NotebookViewModel @Inject constructor( _uiState.update { currentState -> currentState.copy(selectedCourse = course) } + updateJob?.cancel() courseId = course?.courseId loadData(courseId = courseId) } @@ -149,6 +173,7 @@ class NotebookViewModel @Inject constructor( if (courseId != this.courseId || objectTypeAndId != this.objectTypeAndId) { this.courseId = courseId this.objectTypeAndId = objectTypeAndId + updateJob?.cancel() loadData() } } @@ -157,25 +182,28 @@ class NotebookViewModel @Inject constructor( if (courseId != this.courseId) { this.courseId = courseId this.objectTypeAndId = objectTypeAndId + updateJob?.cancel() loadData() } } - private fun updateScreenState() { + private fun removeCourseFilterIfNeeded() { if (courseId != null) { - _uiState.update { it.copy(showTopBar = false) } - if (objectTypeAndId != null) { - _uiState.update { it.copy(showFilters = false) } - } else { - _uiState.update { it.copy(showFilters = true) } - } - } else { - _uiState.update { it.copy(showTopBar = true, showFilters = true) } + _uiState.update { it.copy(showCourseFilter = false) } } } + fun updateScreenState( + showNoteTypeFilter: Boolean = true, + showCourseFilter: Boolean = true, + showTopBar: Boolean = false + ) { + _uiState.update { it.copy(showTopBar = showTopBar, showCourseFilter = showCourseFilter, showNoteTypeFilter = showNoteTypeFilter) } + } + private fun getObjectTypeAndId(savedStateHandle: SavedStateHandle): Pair? { - val objectType = savedStateHandle.get(NotebookRoute.Notebook.OBJECT_TYPE) ?: return null + val objectType = + savedStateHandle.get(NotebookRoute.Notebook.OBJECT_TYPE) ?: return null val objectId = savedStateHandle.get(NotebookRoute.Notebook.OBJECT_ID) ?: return null return Pair(objectType, objectId) } From fb09d481adba70850d34edea0c80339bb3877fd2 Mon Sep 17 00:00:00 2001 From: "andras.maczak" Date: Thu, 13 Nov 2025 15:21:04 +0100 Subject: [PATCH 07/33] notebook ui changes --- .../features/notebook/NotebookScreen.kt | 57 ++++++++++++------- libs/horizon/src/main/res/values/strings.xml | 4 ++ 2 files changed, 40 insertions(+), 21 deletions(-) diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt index 528b90b48a..388ac3e55f 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt @@ -135,7 +135,7 @@ fun NotebookScreen( contentPadding = PaddingValues( start = 16.dp, end = 16.dp, - top = 16.dp, + top = 2.dp, bottom = 16.dp ) ) { @@ -145,10 +145,10 @@ fun NotebookScreen( LoadingContent() } } else if (state.notes.isEmpty()) { - if (state.selectedCourse != null || state.selectedFilter != null) { - - } else { - item { + item { + if (state.selectedCourse != null || state.selectedFilter != null) { + EmptyFilteredContent() + } else { EmptyContent() } } @@ -390,25 +390,40 @@ private fun NoteContent( @Composable private fun EmptyContent(modifier: Modifier = Modifier) { - Box( - contentAlignment = Alignment.Center, + Column( + horizontalAlignment = Alignment.Start, modifier = modifier - .fillMaxWidth() - .horizonShadow( - elevation = HorizonElevation.level4, - shape = HorizonCornerRadius.level2, - clip = true - ) - .background( - color = HorizonColors.PrimitivesWhite.white10(), - shape = HorizonCornerRadius.level2, - ) ) { Text( - text = stringResource(R.string.notebookEmptyContentMessage), + text = stringResource(R.string.notesEmptyContentTitle), + style = HorizonTypography.sh2, + color = HorizonColors.Text.body() + ) + HorizonSpace(size = SpaceSize.SPACE_8) + Text( + text = stringResource(R.string.notesEmptyContentBody), + style = HorizonTypography.p1, + color = HorizonColors.Text.dataPoint() + ) + } +} + +@Composable +private fun EmptyFilteredContent(modifier: Modifier = Modifier) { + Column( + horizontalAlignment = Alignment.Start, + modifier = modifier + ) { + Text( + text = stringResource(R.string.notesEmptyFilteredContentTitle), + style = HorizonTypography.sh2, + color = HorizonColors.Text.body() + ) + HorizonSpace(size = SpaceSize.SPACE_8) + Text( + text = stringResource(R.string.notesEmptyFilteredContentBody), style = HorizonTypography.p1, - color = HorizonColors.Text.body(), - modifier = Modifier.padding(vertical = 40.dp, horizontal = 36.dp) + color = HorizonColors.Text.dataPoint() ) } } @@ -460,4 +475,4 @@ private fun NotebookScreenPreview() { mainNavController = NavHostController(LocalContext.current), state = state ) -} \ No newline at end of file +} diff --git a/libs/horizon/src/main/res/values/strings.xml b/libs/horizon/src/main/res/values/strings.xml index 7fa793f0ea..600e44abc4 100644 --- a/libs/horizon/src/main/res/values/strings.xml +++ b/libs/horizon/src/main/res/values/strings.xml @@ -444,4 +444,8 @@ Previous item %1$d of %2$d Show more + Start capturing your notes + Your notes from learning materials will appear here. Highlight key ideas, reflections, or questions as you learn—they\'ll all be saved in your Notebook for easy review later. + Nothing here yet + Adjust your filters or create a new note to get started. \ No newline at end of file From d0b9ac43b4b0f4e86d64a24ad3fd1c30c2136705 Mon Sep 17 00:00:00 2001 From: "andras.maczak" Date: Thu, 13 Nov 2025 20:07:53 +0100 Subject: [PATCH 08/33] notebook unit tests --- .../notebook/NotebookRepositoryTest.kt | 73 +++++++++- .../notebook/NotebookViewModelTest.kt | 127 ++++++++++++++++-- 2 files changed, 187 insertions(+), 13 deletions(-) diff --git a/libs/horizon/src/test/java/com/instructure/horizon/features/notebook/NotebookRepositoryTest.kt b/libs/horizon/src/test/java/com/instructure/horizon/features/notebook/NotebookRepositoryTest.kt index 9f53185ef9..9780e4169f 100644 --- a/libs/horizon/src/test/java/com/instructure/horizon/features/notebook/NotebookRepositoryTest.kt +++ b/libs/horizon/src/test/java/com/instructure/horizon/features/notebook/NotebookRepositoryTest.kt @@ -17,7 +17,12 @@ package com.instructure.horizon.features.notebook import com.apollographql.apollo.api.Optional +import com.instructure.canvasapi2.managers.graphql.horizon.CourseWithProgress +import com.instructure.canvasapi2.managers.graphql.horizon.HorizonGetCoursesManager import com.instructure.canvasapi2.managers.graphql.horizon.redwood.RedwoodApiManager +import com.instructure.canvasapi2.models.User +import com.instructure.canvasapi2.utils.ApiPrefs +import com.instructure.canvasapi2.utils.DataResult import com.instructure.horizon.features.notebook.common.model.NotebookType import com.instructure.redwood.QueryNotesQuery import com.instructure.redwood.type.LearningObjectFilter @@ -25,14 +30,24 @@ import com.instructure.redwood.type.NoteFilterInput import com.instructure.redwood.type.OrderDirection import io.mockk.coEvery import io.mockk.coVerify +import io.mockk.every import io.mockk.mockk import junit.framework.TestCase.assertEquals import junit.framework.TestCase.assertNotNull +import junit.framework.TestCase.assertTrue import kotlinx.coroutines.test.runTest +import org.junit.Before import org.junit.Test class NotebookRepositoryTest { private val redwoodApiManager: RedwoodApiManager = mockk(relaxed = true) + private val horizonGetCoursesManager: HorizonGetCoursesManager = mockk(relaxed = true) + private val apiPrefs: ApiPrefs = mockk(relaxed = true) + + @Before + fun setup() { + every { apiPrefs.user } returns User(id = 123L) + } @Test fun `Test successful notes retrieval with filter`() = runTest { @@ -141,7 +156,63 @@ class NotebookRepositoryTest { ), any(), any(), any(), any(), any()) } } + @Test + fun `Test error handling for notes retrieval`() = runTest { + coEvery { redwoodApiManager.getNotes(any(), any(), any(), any(), any(), any()) } throws Exception("Network error") + + try { + getRepository().getNotes() + junit.framework.TestCase.fail("Expected exception to be thrown") + } catch (e: Exception) { + assertEquals("Network error", e.message) + } + } + + @Test + fun `Test getCourses returns success`() = runTest { + val mockCourses = listOf( + mockk(relaxed = true) + ) + coEvery { horizonGetCoursesManager.getCoursesWithProgress(any(), any()) } returns DataResult.Success(mockCourses) + + val result = getRepository().getCourses() + + assertTrue(result is DataResult.Success) + assertEquals(mockCourses, (result as DataResult.Success).data) + } + + @Test + fun `Test getCourses with forceNetwork true`() = runTest { + val mockCourses = listOf(mockk(relaxed = true)) + coEvery { horizonGetCoursesManager.getCoursesWithProgress(any(), any()) } returns DataResult.Success(mockCourses) + + getRepository().getCourses(forceNetwork = true) + + coVerify { horizonGetCoursesManager.getCoursesWithProgress(userId = 123L, forceNetwork = true) } + } + + @Test + fun `Test getCourses with forceNetwork false`() = runTest { + val mockCourses = listOf(mockk(relaxed = true)) + coEvery { horizonGetCoursesManager.getCoursesWithProgress(any(), any()) } returns DataResult.Success(mockCourses) + + getRepository().getCourses(forceNetwork = false) + + coVerify { horizonGetCoursesManager.getCoursesWithProgress(userId = 123L, forceNetwork = false) } + } + + @Test + fun `Test getCourses handles missing user`() = runTest { + every { apiPrefs.user } returns null + val mockCourses = listOf(mockk(relaxed = true)) + coEvery { horizonGetCoursesManager.getCoursesWithProgress(any(), any()) } returns DataResult.Success(mockCourses) + + getRepository().getCourses() + + coVerify { horizonGetCoursesManager.getCoursesWithProgress(userId = 0L, forceNetwork = false) } + } + private fun getRepository(): NotebookRepository { - return NotebookRepository(redwoodApiManager) + return NotebookRepository(redwoodApiManager, horizonGetCoursesManager, apiPrefs) } } diff --git a/libs/horizon/src/test/java/com/instructure/horizon/features/notebook/NotebookViewModelTest.kt b/libs/horizon/src/test/java/com/instructure/horizon/features/notebook/NotebookViewModelTest.kt index d58ae47e90..a6db5f613e 100644 --- a/libs/horizon/src/test/java/com/instructure/horizon/features/notebook/NotebookViewModelTest.kt +++ b/libs/horizon/src/test/java/com/instructure/horizon/features/notebook/NotebookViewModelTest.kt @@ -17,14 +17,19 @@ package com.instructure.horizon.features.notebook import androidx.lifecycle.SavedStateHandle +import com.instructure.canvasapi2.managers.graphql.horizon.CourseWithProgress +import com.instructure.canvasapi2.utils.DataResult import com.instructure.horizon.features.notebook.common.model.NotebookType import com.instructure.redwood.QueryNotesQuery import io.mockk.coEvery import io.mockk.coVerify +import io.mockk.every import io.mockk.mockk import io.mockk.unmockkAll import junit.framework.TestCase.assertEquals import junit.framework.TestCase.assertFalse +import junit.framework.TestCase.assertNotNull +import junit.framework.TestCase.assertNull import junit.framework.TestCase.assertTrue import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi @@ -74,6 +79,7 @@ class NotebookViewModelTest { fun setup() { Dispatchers.setMain(testDispatcher) coEvery { repository.getNotes(any(), any(), any(), any(), any(), any(), any()) } returns testNotes + coEvery { repository.getCourses(any()) } returns DataResult.Success(emptyList()) } @After @@ -112,7 +118,6 @@ class NotebookViewModelTest { val viewModel = getViewModel() assertTrue(viewModel.uiState.value.hasNextPage) - assertFalse(viewModel.uiState.value.hasPreviousPage) } @Test @@ -144,28 +149,126 @@ class NotebookViewModelTest { } @Test - fun `Test load previous page uses start cursor`() = runTest { - val notesWithPrevious = testNotes.copy( - pageInfo = testNotes.pageInfo.copy(hasPreviousPage = true) + fun `Test update course id reloads data`() = runTest { + val viewModel = getViewModel() + + viewModel.updateFilters(123L) + viewModel.updateFilters(1234L) + viewModel.updateFilters(123L) + + coVerify(exactly = 2) { repository.getNotes(any(), any(), any(), any(), 123L, any(), any()) } + } + + @Test + fun `Test loadCourses success populates courses`() = runTest { + val mockCourses = listOf( + mockk(relaxed = true), + mockk(relaxed = true) ) - coEvery { repository.getNotes(any(), any(), any(), any(), any(), any(), any()) } returns notesWithPrevious + coEvery { repository.getCourses(any()) } returns DataResult.Success(mockCourses) val viewModel = getViewModel() - viewModel.uiState.value.loadPreviousPage() + assertEquals(mockCourses, viewModel.uiState.value.courses) + } + + @Test + fun `Test loadCourses failure sets empty courses`() = runTest { + coEvery { repository.getCourses(any()) } returns DataResult.Fail() - coVerify { repository.getNotes(after = null, before = "startCursor1", any(), any(), any(), any(), any()) } + val viewModel = getViewModel() + + assertTrue(viewModel.uiState.value.courses.isEmpty()) } @Test - fun `Test update course id reloads data`() = runTest { + fun `Test onCourseSelected updates state and reloads data`() = runTest { + val mockCourse = mockk(relaxed = true) { + every { courseId } returns 123L + } val viewModel = getViewModel() - viewModel.updateFilters(123L) - viewModel.updateFilters(1234L) - viewModel.updateFilters(123L) + viewModel.uiState.value.onCourseSelected(mockCourse) - coVerify(exactly = 2) { repository.getNotes(any(), any(), any(), any(), 123L, any(), any()) } + assertEquals(mockCourse, viewModel.uiState.value.selectedCourse) + coVerify(atLeast = 1) { repository.getNotes(any(), any(), any(), any(), 123L, any(), any()) } + } + + @Test + fun `Test onCourseSelected with null clears course`() = runTest { + val viewModel = getViewModel() + + viewModel.uiState.value.onCourseSelected(null) + + assertNull(viewModel.uiState.value.selectedCourse) + } + + @Test + fun `Test updateScreenState changes visibility flags`() = runTest { + val viewModel = getViewModel() + + viewModel.updateScreenState( + showNoteTypeFilter = false, + showCourseFilter = true, + showTopBar = true + ) + + assertFalse(viewModel.uiState.value.showNoteTypeFilter) + assertTrue(viewModel.uiState.value.showCourseFilter) + assertTrue(viewModel.uiState.value.showTopBar) + } + + @Test + fun `Test updateContent with different courseId reloads data`() = runTest { + val viewModel = getViewModel() + + viewModel.uiState.value.updateContent(456L, Pair("Assignment", "123")) + + coVerify(atLeast = 1) { repository.getNotes(any(), any(), any(), any(), 456L, Pair("Assignment", "123"), any()) } + } + + @Test + fun `Test updateContent with same courseId does not reload data`() = runTest { + val viewModel = getViewModel() + + viewModel.uiState.value.updateContent(null, null) + + coVerify(exactly = 1) { repository.getNotes(any(), any(), any(), any(), any(), any(), any()) } + } + + @Test + fun `Test loadNextPage triggers with valid next page`() = runTest { + coEvery { repository.getNotes(any(), any(), any(), any(), any(), any(), any()) } returns testNotes.copy( + pageInfo = testNotes.pageInfo.copy(hasNextPage = true) + ) + val viewModel = getViewModel() + + viewModel.uiState.value.loadNextPage() + + coVerify(atLeast = 2) { repository.getNotes(any(), any(), any(), any(), any(), any(), any()) } + } + + @Test + fun `Test loadNextPage does not trigger when no next page`() = runTest { + val notesWithoutNext = testNotes.copy( + pageInfo = testNotes.pageInfo.copy(hasNextPage = false) + ) + coEvery { repository.getNotes(any(), any(), any(), any(), any(), any(), any()) } returns notesWithoutNext + + val viewModel = getViewModel() + + viewModel.uiState.value.loadNextPage() + + coVerify(exactly = 1) { repository.getNotes(any(), any(), any(), any(), any(), any(), any()) } + } + + @Test + fun `Test course filter is hidden when courseId is present`() = runTest { + savedStateHandle["courseId"] = "123" + + val viewModel = getViewModel() + + assertFalse(viewModel.uiState.value.showCourseFilter) } private fun getViewModel(): NotebookViewModel { From b32296253ccbda6d16f06b469ac425c5b494882d Mon Sep 17 00:00:00 2001 From: "andras.maczak" Date: Thu, 13 Nov 2025 20:08:27 +0100 Subject: [PATCH 09/33] removed unused import --- .../horizon/features/notebook/NotebookViewModelTest.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/libs/horizon/src/test/java/com/instructure/horizon/features/notebook/NotebookViewModelTest.kt b/libs/horizon/src/test/java/com/instructure/horizon/features/notebook/NotebookViewModelTest.kt index a6db5f613e..b3e8697f02 100644 --- a/libs/horizon/src/test/java/com/instructure/horizon/features/notebook/NotebookViewModelTest.kt +++ b/libs/horizon/src/test/java/com/instructure/horizon/features/notebook/NotebookViewModelTest.kt @@ -28,7 +28,6 @@ import io.mockk.mockk import io.mockk.unmockkAll import junit.framework.TestCase.assertEquals import junit.framework.TestCase.assertFalse -import junit.framework.TestCase.assertNotNull import junit.framework.TestCase.assertNull import junit.framework.TestCase.assertTrue import kotlinx.coroutines.Dispatchers From 75ce829f3dcd18bc0b52d449e98bb6d9c0f60594 Mon Sep 17 00:00:00 2001 From: "andras.maczak" Date: Tue, 18 Nov 2025 13:21:06 +0100 Subject: [PATCH 10/33] fix PR findings --- .../features/notebook/NotebookScreen.kt | 104 ++++++++++++++++-- .../features/notebook/NotebookUiState.kt | 1 + .../features/notebook/NotebookViewModel.kt | 6 +- .../horizonui/molecules/DropdownChip.kt | 7 +- libs/horizon/src/main/res/values/strings.xml | 2 + 5 files changed, 108 insertions(+), 12 deletions(-) diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt index 388ac3e55f..aaf4cb90eb 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt @@ -74,6 +74,7 @@ import com.instructure.horizon.horizonui.molecules.DropdownChip import com.instructure.horizon.horizonui.molecules.DropdownItem import com.instructure.horizon.horizonui.molecules.HorizonDivider import com.instructure.horizon.horizonui.molecules.Spinner +import com.instructure.horizon.horizonui.organisms.CollapsableHeaderScreen import com.instructure.horizon.navigation.MainNavigationRoute import com.instructure.pandautils.utils.ViewStyler import com.instructure.pandautils.utils.getActivityOrNull @@ -94,9 +95,9 @@ fun NotebookScreen( } val scrollState = rememberLazyListState() - Scaffold( - containerColor = HorizonColors.Surface.pagePrimary(), - topBar = { + CollapsableHeaderScreen( + modifier = Modifier.background(HorizonColors.Surface.pagePrimary()), + headerContent = { if (state.showTopBar) { NotebookAppBar( navigateBack = { mainNavController.popBackStack() }, @@ -104,12 +105,12 @@ fun NotebookScreen( ) } }, - ) { padding -> + bodyContent = { Column( horizontalAlignment = Alignment.CenterHorizontally, modifier = Modifier .fillMaxSize() - .padding(paddingValues = padding) + //.padding(paddingValues = padding) ) { if ((state.showNoteTypeFilter || state.showCourseFilter) && (state.courses.isNotEmpty() || state.selectedCourse != null || state.selectedFilter != null)) { FilterContent( @@ -144,6 +145,10 @@ fun NotebookScreen( item { LoadingContent() } + } else if (state.isError) { + item { + ErrorContent() + } } else if (state.notes.isEmpty()) { item { if (state.selectedCourse != null || state.selectedFilter != null) { @@ -212,7 +217,7 @@ fun NotebookScreen( } } } - } + }) } @Composable @@ -301,7 +306,8 @@ private fun FilterContent( onItemSelected = { item -> onCourseSelected(item?.value) }, placeholder = stringResource(R.string.notebookFilterCoursePlaceholder), dropdownWidth = 178.dp, - verticalPadding = 6.dp + verticalPadding = 6.dp, + modifier = Modifier.weight(1f, false) ) } @@ -392,7 +398,7 @@ private fun NoteContent( private fun EmptyContent(modifier: Modifier = Modifier) { Column( horizontalAlignment = Alignment.Start, - modifier = modifier + modifier = modifier.fillMaxWidth() ) { Text( text = stringResource(R.string.notesEmptyContentTitle), @@ -412,7 +418,7 @@ private fun EmptyContent(modifier: Modifier = Modifier) { private fun EmptyFilteredContent(modifier: Modifier = Modifier) { Column( horizontalAlignment = Alignment.Start, - modifier = modifier + modifier = modifier.fillMaxWidth() ) { Text( text = stringResource(R.string.notesEmptyFilteredContentTitle), @@ -428,6 +434,26 @@ private fun EmptyFilteredContent(modifier: Modifier = Modifier) { } } +@Composable +private fun ErrorContent(modifier: Modifier = Modifier) { + Column( + horizontalAlignment = Alignment.Start, + modifier = modifier.fillMaxWidth() + ) { + Text( + text = stringResource(R.string.notesErrorContentTitle), + style = HorizonTypography.sh2, + color = HorizonColors.Text.body() + ) + HorizonSpace(size = SpaceSize.SPACE_8) + Text( + text = stringResource(R.string.notesErrorContentBody), + style = HorizonTypography.p1, + color = HorizonColors.Text.dataPoint() + ) + } +} + @Composable @Preview private fun NotebookScreenPreview() { @@ -476,3 +502,63 @@ private fun NotebookScreenPreview() { state = state ) } + +@Composable +@Preview +private fun NotebookScreenEmptyPreview() { + ContextKeeper.appContext = LocalContext.current + val state = NotebookUiState( + isLoading = false, + showNoteTypeFilter = true, + showCourseFilter = true, + showTopBar = true, + notes = emptyList(), + updateContent = { _, _ -> } + ) + + NotebookScreen( + mainNavController = NavHostController(LocalContext.current), + state = state + ) +} + +@Composable +@Preview +private fun NotebookScreenEmptyFilteredPreview() { + ContextKeeper.appContext = LocalContext.current + val state = NotebookUiState( + isLoading = false, + showNoteTypeFilter = true, + showCourseFilter = true, + showTopBar = true, + selectedFilter = NotebookType.Important, + notes = emptyList(), + updateContent = { _, _ -> } + ) + + NotebookScreen( + mainNavController = NavHostController(LocalContext.current), + state = state + ) +} + +@Composable +@Preview +private fun NotebookScreenErrorPreview() { + ContextKeeper.appContext = LocalContext.current + val state = NotebookUiState( + isError = true, + isLoading = false, + showNoteTypeFilter = true, + showCourseFilter = true, + showTopBar = true, + selectedFilter = NotebookType.Important, + notes = emptyList(), + updateContent = { _, _ -> } + ) + + NotebookScreen( + mainNavController = NavHostController(LocalContext.current), + state = state + ) +} diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookUiState.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookUiState.kt index c15e544929..c89696d7d9 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookUiState.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookUiState.kt @@ -23,6 +23,7 @@ import com.instructure.horizon.features.notebook.common.model.NotebookType data class NotebookUiState( val isLoading: Boolean = true, val isLoadingMore: Boolean = false, + val isError: Boolean = false, val selectedFilter: NotebookType? = null, val onFilterSelected: (NotebookType?) -> Unit = {}, val selectedCourse: CourseWithProgress? = null, diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookViewModel.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookViewModel.kt index fb4ce50d6c..5ea12422cf 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookViewModel.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookViewModel.kt @@ -96,9 +96,9 @@ class NotebookViewModel @Inject constructor( updateJob = viewModelScope.tryLaunch { _uiState.update { if (isLoadingMore) { - it.copy(isLoadingMore = true) + it.copy(isLoadingMore = true, isError = false) } else { - it.copy(isLoading = true) + it.copy(isLoading = true, isError = false) } } @@ -129,6 +129,7 @@ class NotebookViewModel @Inject constructor( _uiState.update { it.copy( + isError = false, isLoading = false, isLoadingMore = false, notes = if (isLoadingMore) oldNotes + newNotes else newNotes, @@ -138,6 +139,7 @@ class NotebookViewModel @Inject constructor( } catch { _uiState.update { it.copy( + isError = true, isLoading = false, isLoadingMore = false, hasNextPage = false, diff --git a/libs/horizon/src/main/java/com/instructure/horizon/horizonui/molecules/DropdownChip.kt b/libs/horizon/src/main/java/com/instructure/horizon/horizonui/molecules/DropdownChip.kt index 3e537fb35a..66e3dcdf65 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/horizonui/molecules/DropdownChip.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/horizonui/molecules/DropdownChip.kt @@ -47,6 +47,7 @@ import androidx.compose.ui.semantics.clearAndSetSemantics import androidx.compose.ui.semantics.contentDescription import androidx.compose.ui.semantics.role import androidx.compose.ui.semantics.stateDescription +import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp @@ -124,7 +125,11 @@ fun DropdownChip( text = selectedItem?.label ?: placeholder, style = HorizonTypography.p2, color = contentColor, - modifier = Modifier.padding( + maxLines = 1, + overflow = TextOverflow.Ellipsis, + modifier = Modifier + .weight(1f, false) + .padding( end = 2.dp, top = verticalPadding, bottom = verticalPadding diff --git a/libs/horizon/src/main/res/values/strings.xml b/libs/horizon/src/main/res/values/strings.xml index 37f8261aa2..c284711157 100644 --- a/libs/horizon/src/main/res/values/strings.xml +++ b/libs/horizon/src/main/res/values/strings.xml @@ -448,4 +448,6 @@ Your notes from learning materials will appear here. Highlight key ideas, reflections, or questions as you learn—they\'ll all be saved in your Notebook for easy review later. Nothing here yet Adjust your filters or create a new note to get started. + Failed to load Notebook + Please try again. \ No newline at end of file From c5181053d8e4e00828344725a7b278221a70625c Mon Sep 17 00:00:00 2001 From: "andras.maczak" Date: Wed, 19 Nov 2025 15:29:24 +0100 Subject: [PATCH 11/33] pull to refresh logic to notebook --- .../learn/course/note/CourseNotesScreen.kt | 5 +- .../features/notebook/NotebookScreen.kt | 129 ++++++------------ .../features/notebook/NotebookUiState.kt | 1 + .../features/notebook/NotebookViewModel.kt | 23 +++- .../notebook/navigation/NotebookNavigation.kt | 3 +- 5 files changed, 64 insertions(+), 97 deletions(-) diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/learn/course/note/CourseNotesScreen.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/learn/course/note/CourseNotesScreen.kt index 68ed097656..b370104290 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/learn/course/note/CourseNotesScreen.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/learn/course/note/CourseNotesScreen.kt @@ -19,8 +19,6 @@ package com.instructure.horizon.features.learn.course.note import androidx.compose.foundation.layout.Box import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect -import androidx.compose.runtime.collectAsState -import androidx.compose.runtime.getValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.hilt.navigation.compose.hiltViewModel @@ -35,7 +33,6 @@ fun CourseNotesScreen( modifier: Modifier = Modifier ) { val viewModel: NotebookViewModel = hiltViewModel() - val state by viewModel.uiState.collectAsState() LaunchedEffect(courseId) { viewModel.updateFilters(courseId) @@ -48,7 +45,7 @@ fun CourseNotesScreen( ) { NotebookScreen( mainNavController, - state + viewModel ) } } \ No newline at end of file diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt index aaf4cb90eb..a496ecebd7 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt @@ -14,6 +14,8 @@ * along with this program. If not, see . * */ +@file:OptIn(androidx.compose.material3.ExperimentalMaterial3Api::class) + package com.instructure.horizon.features.notebook import androidx.compose.foundation.background @@ -32,8 +34,13 @@ import androidx.compose.foundation.lazy.items import androidx.compose.foundation.lazy.rememberLazyListState import androidx.compose.material3.Scaffold import androidx.compose.material3.Text +import androidx.compose.material3.pulltorefresh.PullToRefreshBox +import androidx.compose.material3.pulltorefresh.PullToRefreshDefaults.Indicator +import androidx.compose.material3.pulltorefresh.rememberPullToRefreshState import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.collectAsState +import androidx.compose.runtime.getValue import androidx.compose.runtime.remember import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier @@ -84,8 +91,9 @@ import java.util.Date @Composable fun NotebookScreen( mainNavController: NavHostController, - state: NotebookUiState, + viewModel: NotebookViewModel, ) { + val state by viewModel.uiState.collectAsState() val activity = LocalContext.current.getActivityOrNull() LaunchedEffect(Unit) { if (activity != null) ViewStyler.setStatusBarColor( @@ -95,6 +103,8 @@ fun NotebookScreen( } val scrollState = rememberLazyListState() + val pullToRefreshState = rememberPullToRefreshState() + CollapsableHeaderScreen( modifier = Modifier.background(HorizonColors.Surface.pagePrimary()), headerContent = { @@ -106,6 +116,22 @@ fun NotebookScreen( } }, bodyContent = { + PullToRefreshBox( + isRefreshing = state.isRefreshing, + onRefresh = { viewModel.refresh() }, + state = pullToRefreshState, + indicator = { + Indicator( + modifier = Modifier + .align(Alignment.TopCenter) + .padding(top = if (state.showTopBar) 56.dp else 0.dp), + isRefreshing = state.isRefreshing, + containerColor = HorizonColors.Surface.pageSecondary(), + color = HorizonColors.Surface.institution(), + state = pullToRefreshState + ) + } + ) { Column( horizontalAlignment = Alignment.CenterHorizontally, modifier = Modifier @@ -217,6 +243,7 @@ fun NotebookScreen( } } } + } }) } @@ -458,107 +485,41 @@ private fun ErrorContent(modifier: Modifier = Modifier) { @Preview private fun NotebookScreenPreview() { ContextKeeper.appContext = LocalContext.current - val state = NotebookUiState( - isLoading = false, - showNoteTypeFilter = true, - showCourseFilter = true, - showTopBar = true, - selectedFilter = NotebookType.Important, - notes = listOf( - Note( - id = "1", - courseId = 123L, - objectId = "456", - objectType = NoteObjectType.PAGE, - userText = "This is a note about an assignment.", - highlightedText = NoteHighlightedData( - "Important part of the assignment.", - NoteHighlightedDataRange(0, 0, "", ""), - NoteHighlightedDataTextPosition(0, 0) - ), - updatedAt = Date(), - type = NotebookType.Important - ), - Note( - id = "2", - courseId = 123L, - objectId = "789", - objectType = NoteObjectType.PAGE, - userText = "This is a note about another assignment.", - highlightedText = NoteHighlightedData( - "Confusing part of the assignment.", - NoteHighlightedDataRange(0, 0, "", ""), - NoteHighlightedDataTextPosition(0, 0) - ), - updatedAt = Date(), - type = NotebookType.Confusing - ) - ), - updateContent = { _, _ -> } - ) - - NotebookScreen( - mainNavController = NavHostController(LocalContext.current), - state = state - ) + FilterContentPreview() } @Composable @Preview private fun NotebookScreenEmptyPreview() { ContextKeeper.appContext = LocalContext.current - val state = NotebookUiState( - isLoading = false, - showNoteTypeFilter = true, - showCourseFilter = true, - showTopBar = true, - notes = emptyList(), - updateContent = { _, _ -> } - ) - - NotebookScreen( - mainNavController = NavHostController(LocalContext.current), - state = state - ) + EmptyContent() } @Composable @Preview private fun NotebookScreenEmptyFilteredPreview() { ContextKeeper.appContext = LocalContext.current - val state = NotebookUiState( - isLoading = false, - showNoteTypeFilter = true, - showCourseFilter = true, - showTopBar = true, - selectedFilter = NotebookType.Important, - notes = emptyList(), - updateContent = { _, _ -> } - ) - - NotebookScreen( - mainNavController = NavHostController(LocalContext.current), - state = state - ) + EmptyFilteredContent() } @Composable @Preview private fun NotebookScreenErrorPreview() { ContextKeeper.appContext = LocalContext.current - val state = NotebookUiState( - isError = true, - isLoading = false, - showNoteTypeFilter = true, - showCourseFilter = true, - showTopBar = true, - selectedFilter = NotebookType.Important, - notes = emptyList(), - updateContent = { _, _ -> } - ) + ErrorContent() +} - NotebookScreen( - mainNavController = NavHostController(LocalContext.current), - state = state +@Composable +@Preview +private fun FilterContentPreview() { + ContextKeeper.appContext = LocalContext.current + FilterContent( + selectedFilter = NotebookType.Important, + onFilterSelected = {}, + selectedCourse = null, + onCourseSelected = {}, + courses = emptyList(), + showNoteTypeFilter = true, + showCourseFilter = true ) } diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookUiState.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookUiState.kt index c89696d7d9..83384b62dc 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookUiState.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookUiState.kt @@ -23,6 +23,7 @@ import com.instructure.horizon.features.notebook.common.model.NotebookType data class NotebookUiState( val isLoading: Boolean = true, val isLoadingMore: Boolean = false, + val isRefreshing: Boolean = false, val isError: Boolean = false, val selectedFilter: NotebookType? = null, val onFilterSelected: (NotebookType?) -> Unit = {}, diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookViewModel.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookViewModel.kt index 5ea12422cf..7deff2f94d 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookViewModel.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookViewModel.kt @@ -74,9 +74,9 @@ class NotebookViewModel @Inject constructor( loadData() } - private fun loadCourses() { + private fun loadCourses(forceNetwork: Boolean = false) { viewModelScope.launch { - when (val result = repository.getCourses()) { + when (val result = repository.getCourses(forceNetwork)) { is DataResult.Success -> { _uiState.update { it.copy(courses = result.data) } } @@ -88,17 +88,24 @@ class NotebookViewModel @Inject constructor( } } + fun refresh() { + updateJob?.cancel() + loadCourses(forceNetwork = true) + loadData(isRefreshing = true) + } + private fun loadData( after: String? = null, courseId: Long? = this.courseId, - isLoadingMore: Boolean = false + isLoadingMore: Boolean = false, + isRefreshing: Boolean = false ) { updateJob = viewModelScope.tryLaunch { _uiState.update { - if (isLoadingMore) { - it.copy(isLoadingMore = true, isError = false) - } else { - it.copy(isLoading = true, isError = false) + when { + isRefreshing -> it.copy(isRefreshing = true, isError = false) + isLoadingMore -> it.copy(isLoadingMore = true, isError = false) + else -> it.copy(isLoading = true, isError = false) } } @@ -132,6 +139,7 @@ class NotebookViewModel @Inject constructor( isError = false, isLoading = false, isLoadingMore = false, + isRefreshing = false, notes = if (isLoadingMore) oldNotes + newNotes else newNotes, hasNextPage = notesResponse.pageInfo.hasNextPage, ) @@ -142,6 +150,7 @@ class NotebookViewModel @Inject constructor( isError = true, isLoading = false, isLoadingMore = false, + isRefreshing = false, hasNextPage = false, ) } diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/navigation/NotebookNavigation.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/navigation/NotebookNavigation.kt index b4d9cc9717..b1997781f4 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/navigation/NotebookNavigation.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/navigation/NotebookNavigation.kt @@ -78,8 +78,7 @@ fun NavGraphBuilder.notebookNavigation( ) ) { val viewModel = hiltViewModel() - val uiState by viewModel.uiState.collectAsState() - NotebookScreen(navController, uiState) + NotebookScreen(navController, viewModel) } composable { val viewModel = hiltViewModel() From f9b8b368814e6b68b78b34ddb2ed0a98468adcc3 Mon Sep 17 00:00:00 2001 From: domonkosadam Date: Mon, 24 Nov 2025 15:00:21 +0100 Subject: [PATCH 12/33] Fix findings --- .../horizon/redwood/RedwoodApiManager.kt | 3 +- .../features/notebook/NotebookRepository.kt | 9 +- .../features/notebook/NotebookScreen.kt | 307 +++++++----------- .../features/notebook/NotebookUiState.kt | 6 +- .../features/notebook/NotebookViewModel.kt | 174 ++++------ libs/horizon/src/main/res/values/strings.xml | 2 +- .../notebook/NotebookRepositoryTest.kt | 20 +- .../notebook/NotebookViewModelTest.kt | 36 +- 8 files changed, 203 insertions(+), 354 deletions(-) diff --git a/libs/canvas-api-2/src/main/java/com/instructure/canvasapi2/managers/graphql/horizon/redwood/RedwoodApiManager.kt b/libs/canvas-api-2/src/main/java/com/instructure/canvasapi2/managers/graphql/horizon/redwood/RedwoodApiManager.kt index c9e168c76c..3d51537049 100644 --- a/libs/canvas-api-2/src/main/java/com/instructure/canvasapi2/managers/graphql/horizon/redwood/RedwoodApiManager.kt +++ b/libs/canvas-api-2/src/main/java/com/instructure/canvasapi2/managers/graphql/horizon/redwood/RedwoodApiManager.kt @@ -99,6 +99,7 @@ class RedwoodApiManager @Inject constructor( after: String? = null, before: String? = null, orderBy: OrderByInput? = null, + forceNetwork: Boolean = false ): QueryNotesQuery.Notes { val query = QueryNotesQuery( filter = Optional.presentIfNotNull(filter), @@ -109,7 +110,7 @@ class RedwoodApiManager @Inject constructor( orderBy = Optional.presentIfNotNull(orderBy), ) val result = redwoodClient - .enqueueQuery(query) + .enqueueQuery(query, forceNetwork) .dataAssertNoErrors.notes return result diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookRepository.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookRepository.kt index 32e17571c7..45c4d5d222 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookRepository.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookRepository.kt @@ -42,7 +42,8 @@ class NotebookRepository @Inject constructor( filterType: NotebookType? = null, courseId: Long? = null, objectTypeAndId: Pair? = null, - orderDirection: OrderDirection? = null + orderDirection: OrderDirection? = null, + forceNetwork: Boolean = false ): QueryNotesQuery.Notes { val filterInput = NoteFilterInput( reactions = if (filterType != null) { @@ -73,14 +74,16 @@ class NotebookRepository @Inject constructor( lastN = itemCount, before = before, filter = filterInput, - orderBy = orderByInput + orderBy = orderByInput, + forceNetwork = forceNetwork ) } else { redwoodApiManager.getNotes( firstN = itemCount, after = after, filter = filterInput, - orderBy = orderByInput + orderBy = orderByInput, + forceNetwork = forceNetwork ) } diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt index a496ecebd7..2f96b7be89 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt @@ -21,7 +21,6 @@ package com.instructure.horizon.features.notebook import androidx.compose.foundation.background import androidx.compose.foundation.clickable import androidx.compose.foundation.layout.Arrangement -import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.PaddingValues import androidx.compose.foundation.layout.Row @@ -30,13 +29,10 @@ import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding import androidx.compose.foundation.lazy.LazyColumn +import androidx.compose.foundation.lazy.LazyListState import androidx.compose.foundation.lazy.items import androidx.compose.foundation.lazy.rememberLazyListState -import androidx.compose.material3.Scaffold import androidx.compose.material3.Text -import androidx.compose.material3.pulltorefresh.PullToRefreshBox -import androidx.compose.material3.pulltorefresh.PullToRefreshDefaults.Indicator -import androidx.compose.material3.pulltorefresh.rememberPullToRefreshState import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.collectAsState @@ -54,10 +50,6 @@ import androidx.compose.ui.unit.dp import androidx.core.content.ContextCompat import androidx.navigation.NavHostController import com.instructure.canvasapi2.managers.graphql.horizon.CourseWithProgress -import com.instructure.canvasapi2.managers.graphql.horizon.redwood.NoteHighlightedData -import com.instructure.canvasapi2.managers.graphql.horizon.redwood.NoteHighlightedDataRange -import com.instructure.canvasapi2.managers.graphql.horizon.redwood.NoteHighlightedDataTextPosition -import com.instructure.canvasapi2.managers.graphql.horizon.redwood.NoteObjectType import com.instructure.canvasapi2.utils.ContextKeeper import com.instructure.horizon.R import com.instructure.horizon.features.notebook.common.composable.NotebookAppBar @@ -82,11 +74,11 @@ import com.instructure.horizon.horizonui.molecules.DropdownItem import com.instructure.horizon.horizonui.molecules.HorizonDivider import com.instructure.horizon.horizonui.molecules.Spinner import com.instructure.horizon.horizonui.organisms.CollapsableHeaderScreen +import com.instructure.horizon.horizonui.platform.LoadingStateWrapper import com.instructure.horizon.navigation.MainNavigationRoute import com.instructure.pandautils.utils.ViewStyler import com.instructure.pandautils.utils.getActivityOrNull import com.instructure.pandautils.utils.localisedFormat -import java.util.Date @Composable fun NotebookScreen( @@ -103,7 +95,6 @@ fun NotebookScreen( } val scrollState = rememberLazyListState() - val pullToRefreshState = rememberPullToRefreshState() CollapsableHeaderScreen( modifier = Modifier.background(HorizonColors.Surface.pagePrimary()), @@ -115,148 +106,108 @@ fun NotebookScreen( ) } }, - bodyContent = { - PullToRefreshBox( - isRefreshing = state.isRefreshing, - onRefresh = { viewModel.refresh() }, - state = pullToRefreshState, - indicator = { - Indicator( + bodyContent = { + LoadingStateWrapper(state.loadingState) { + Column( + horizontalAlignment = Alignment.CenterHorizontally, modifier = Modifier - .align(Alignment.TopCenter) - .padding(top = if (state.showTopBar) 56.dp else 0.dp), - isRefreshing = state.isRefreshing, - containerColor = HorizonColors.Surface.pageSecondary(), - color = HorizonColors.Surface.institution(), - state = pullToRefreshState - ) - } - ) { - Column( - horizontalAlignment = Alignment.CenterHorizontally, - modifier = Modifier - .fillMaxSize() - //.padding(paddingValues = padding) - ) { - if ((state.showNoteTypeFilter || state.showCourseFilter) && (state.courses.isNotEmpty() || state.selectedCourse != null || state.selectedFilter != null)) { - FilterContent( - modifier = Modifier - .clip(HorizonCornerRadius.level5) - .background(HorizonColors.Surface.pageSecondary()), - selectedFilter = state.selectedFilter, - onFilterSelected = state.onFilterSelected, - selectedCourse = state.selectedCourse, - onCourseSelected = state.onCourseSelected, - courses = state.courses, - showNoteTypeFilter = state.showNoteTypeFilter, - showCourseFilter = state.showCourseFilter - ) - } - - Box { - LazyColumn( - state = scrollState, - modifier = Modifier - .fillMaxHeight() - .background(HorizonColors.Surface.pageSecondary()), - contentPadding = PaddingValues( - start = 16.dp, - end = 16.dp, - top = 2.dp, - bottom = 16.dp - ) + .fillMaxSize() ) { + if ((state.showNoteTypeFilter || state.showCourseFilter) && (state.courses.isNotEmpty() || state.selectedCourse != null || state.selectedFilter != null)) { + FilterContent( + state, + scrollState, + Modifier + .clip(HorizonCornerRadius.level5) + .background(HorizonColors.Surface.pageSecondary()), + ) + } - if (state.isLoading) { - item { - LoadingContent() - } - } else if (state.isError) { - item { - ErrorContent() - } - } else if (state.notes.isEmpty()) { - item { - if (state.selectedCourse != null || state.selectedFilter != null) { - EmptyFilteredContent() - } else { - EmptyContent() + LazyColumn( + state = scrollState, + modifier = Modifier + .fillMaxHeight() + .background(HorizonColors.Surface.pageSecondary()), + contentPadding = PaddingValues( + start = 16.dp, + end = 16.dp, + top = 2.dp, + bottom = 16.dp + ) + ) { + if (state.notes.isEmpty()) { + item { + if (state.selectedCourse != null || state.selectedFilter != null) { + EmptyFilteredContent() + } else { + EmptyContent() + } } - } - } else { - items(state.notes) { note -> - Column { - NoteContent(note) { - if (state.navigateToEdit) { - mainNavController.navigate( - NotebookRoute.EditNotebook( - noteId = note.id, - highlightedTextStartOffset = note.highlightedText.range.startOffset, - highlightedTextEndOffset = note.highlightedText.range.endOffset, - highlightedTextStartContainer = note.highlightedText.range.startContainer, - highlightedTextEndContainer = note.highlightedText.range.endContainer, - textSelectionStart = note.highlightedText.textPosition.start, - textSelectionEnd = note.highlightedText.textPosition.end, - highlightedText = note.highlightedText.selectedText, - noteType = note.type.name, - userComment = note.userText + } else { + items(state.notes) { note -> + Column { + NoteContent(note) { + if (state.navigateToEdit) { + mainNavController.navigate( + NotebookRoute.EditNotebook( + noteId = note.id, + highlightedTextStartOffset = note.highlightedText.range.startOffset, + highlightedTextEndOffset = note.highlightedText.range.endOffset, + highlightedTextStartContainer = note.highlightedText.range.startContainer, + highlightedTextEndContainer = note.highlightedText.range.endContainer, + textSelectionStart = note.highlightedText.textPosition.start, + textSelectionEnd = note.highlightedText.textPosition.end, + highlightedText = note.highlightedText.selectedText, + noteType = note.type.name, + userComment = note.userText + ) ) - ) - } else { - mainNavController.navigate( - MainNavigationRoute.ModuleItemSequence( - courseId = note.courseId, - moduleItemAssetType = note.objectType.value, - moduleItemAssetId = note.objectId, + } else { + mainNavController.navigate( + MainNavigationRoute.ModuleItemSequence( + courseId = note.courseId, + moduleItemAssetType = note.objectType.value, + moduleItemAssetId = note.objectId, + ) ) - ) + } } - } - if (state.notes.lastOrNull() != note) { - HorizonSpace(SpaceSize.SPACE_4) + if (state.notes.lastOrNull() != note) { + HorizonSpace(SpaceSize.SPACE_4) + } } } - } - if (state.hasNextPage) { - item { - if (state.isLoadingMore) { - LoadingContent() - } else { - Button( - label = stringResource(R.string.showMore), - height = ButtonHeight.SMALL, - width = ButtonWidth.FILL, - color = ButtonColor.WhiteWithOutline, - onClick = { state.loadNextPage() }, - modifier = Modifier.padding(vertical = 24.dp) - ) + if (state.hasNextPage) { + item { + if (state.isLoadingMore) { + LoadingContent() + } else { + Button( + label = stringResource(R.string.showMore), + height = ButtonHeight.SMALL, + width = ButtonWidth.FILL, + color = ButtonColor.WhiteWithOutline, + onClick = { state.loadNextPage() }, + modifier = Modifier.padding(vertical = 24.dp) + ) + } } } } } } - - if (scrollState.canScrollBackward) { - HorizonDivider() - } } } - } - }) + ) } @Composable private fun FilterContent( + state: NotebookUiState, + scrollState: LazyListState, modifier: Modifier = Modifier, - selectedFilter: NotebookType?, - onFilterSelected: (NotebookType?) -> Unit, - selectedCourse: CourseWithProgress?, - onCourseSelected: (CourseWithProgress?) -> Unit, - courses: List, - showNoteTypeFilter: Boolean = true, - showCourseFilter: Boolean = true, ) { val context = LocalContext.current val defaultBackgroundColor = HorizonColors.PrimitivesGrey.grey12() @@ -293,16 +244,16 @@ private fun FilterContent( } // Course filter items - val allCoursesItem = DropdownItem( - value = null as CourseWithProgress?, + val allCoursesItem = DropdownItem( + value = null, label = context.getString(R.string.notebookFilterCoursePlaceholder), iconRes = null, iconTint = null, backgroundColor = defaultBackgroundColor ) - val courseItems = remember(courses) { - listOf(allCoursesItem) + courses.map { course -> + val courseItems = remember(state.courses) { + listOf(allCoursesItem) + state.courses.map { course -> DropdownItem( value = course, label = course.courseName, @@ -314,41 +265,47 @@ private fun FilterContent( } val selectedTypeItem = - if (selectedFilter == null) allNotesItem else typeItems.find { it.value == selectedFilter } + if (state.selectedFilter == null) allNotesItem else typeItems.find { it.value == state.selectedFilter } val selectedCourseItem = - if (selectedCourse == null) allCoursesItem else courseItems.find { it.value == selectedCourse } - + if (state.selectedCourse == null) allCoursesItem else courseItems.find { it.value == state.selectedCourse } + + + Column { + Row( + modifier = modifier + .fillMaxWidth() + .padding(16.dp), + horizontalArrangement = Arrangement.spacedBy(8.dp) + ) { + if (state.showCourseFilter) { + DropdownChip( + items = courseItems, + selectedItem = selectedCourseItem, + onItemSelected = { item -> state.onCourseSelected(item?.value) }, + placeholder = stringResource(R.string.notebookFilterCoursePlaceholder), + dropdownWidth = 178.dp, + verticalPadding = 6.dp, + modifier = Modifier.weight(1f, false) + ) + } - Row( - modifier = modifier - .fillMaxWidth() - .padding(16.dp), - horizontalArrangement = Arrangement.spacedBy(8.dp) - ) { - if (showCourseFilter) { - DropdownChip( - items = courseItems, - selectedItem = selectedCourseItem, - onItemSelected = { item -> onCourseSelected(item?.value) }, - placeholder = stringResource(R.string.notebookFilterCoursePlaceholder), - dropdownWidth = 178.dp, - verticalPadding = 6.dp, - modifier = Modifier.weight(1f, false) - ) - } + if (state.showNoteTypeFilter) { + DropdownChip( + items = typeItems, + selectedItem = selectedTypeItem, + onItemSelected = { item -> state.onFilterSelected(item?.value) }, + placeholder = stringResource(R.string.notebookFilterTypePlaceholder), + dropdownWidth = 178.dp, + verticalPadding = 6.dp + ) + } + } - if (showNoteTypeFilter) { - DropdownChip( - items = typeItems, - selectedItem = selectedTypeItem, - onItemSelected = { item -> onFilterSelected(item?.value) }, - placeholder = stringResource(R.string.notebookFilterTypePlaceholder), - dropdownWidth = 178.dp, - verticalPadding = 6.dp - ) + if (scrollState.canScrollBackward) { + HorizonDivider() + } } - } } @Composable @@ -481,13 +438,6 @@ private fun ErrorContent(modifier: Modifier = Modifier) { } } -@Composable -@Preview -private fun NotebookScreenPreview() { - ContextKeeper.appContext = LocalContext.current - FilterContentPreview() -} - @Composable @Preview private fun NotebookScreenEmptyPreview() { @@ -508,18 +458,3 @@ private fun NotebookScreenErrorPreview() { ContextKeeper.appContext = LocalContext.current ErrorContent() } - -@Composable -@Preview -private fun FilterContentPreview() { - ContextKeeper.appContext = LocalContext.current - FilterContent( - selectedFilter = NotebookType.Important, - onFilterSelected = {}, - selectedCourse = null, - onCourseSelected = {}, - courses = emptyList(), - showNoteTypeFilter = true, - showCourseFilter = true - ) -} diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookUiState.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookUiState.kt index 83384b62dc..af097acf76 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookUiState.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookUiState.kt @@ -19,12 +19,11 @@ package com.instructure.horizon.features.notebook import com.instructure.canvasapi2.managers.graphql.horizon.CourseWithProgress import com.instructure.horizon.features.notebook.common.model.Note import com.instructure.horizon.features.notebook.common.model.NotebookType +import com.instructure.horizon.horizonui.platform.LoadingState data class NotebookUiState( - val isLoading: Boolean = true, + val loadingState: LoadingState = LoadingState(), val isLoadingMore: Boolean = false, - val isRefreshing: Boolean = false, - val isError: Boolean = false, val selectedFilter: NotebookType? = null, val onFilterSelected: (NotebookType?) -> Unit = {}, val selectedCourse: CourseWithProgress? = null, @@ -33,7 +32,6 @@ data class NotebookUiState( val notes: List = emptyList(), val hasNextPage: Boolean = false, val loadNextPage: () -> Unit = {}, - val updateContent: (Long?, Pair?) -> Unit, val showTopBar: Boolean = false, val showFilters: Boolean = false, val navigateToEdit: Boolean = false, diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookViewModel.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookViewModel.kt index 7deff2f94d..1cd8ad0876 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookViewModel.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookViewModel.kt @@ -20,12 +20,13 @@ import androidx.lifecycle.SavedStateHandle import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import com.instructure.canvasapi2.managers.graphql.horizon.CourseWithProgress -import com.instructure.canvasapi2.utils.DataResult import com.instructure.canvasapi2.utils.weave.catch import com.instructure.canvasapi2.utils.weave.tryLaunch +import com.instructure.horizon.features.notebook.common.model.Note import com.instructure.horizon.features.notebook.common.model.NotebookType import com.instructure.horizon.features.notebook.common.model.mapToNotes import com.instructure.horizon.features.notebook.navigation.NotebookRoute +import com.instructure.horizon.horizonui.platform.LoadingState import com.instructure.redwood.QueryNotesQuery import com.instructure.redwood.type.OrderDirection import dagger.hilt.android.lifecycle.HiltViewModel @@ -33,7 +34,6 @@ import kotlinx.coroutines.Job import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.update -import kotlinx.coroutines.launch import javax.inject.Inject @HiltViewModel @@ -41,8 +41,8 @@ class NotebookViewModel @Inject constructor( private val repository: NotebookRepository, savedStateHandle: SavedStateHandle, ) : ViewModel() { - private var cursorId: String? = null private var pageInfo: QueryNotesQuery.PageInfo? = null + private var loadJob: Job? = null private var courseId: Long? = savedStateHandle.get(NotebookRoute.Notebook.COURSE_ID)?.toLongOrNull() @@ -56,152 +56,112 @@ class NotebookViewModel @Inject constructor( private val _uiState = MutableStateFlow( NotebookUiState( + loadingState = LoadingState( + onSnackbarDismiss = ::onSnackbarDismiss, + onRefresh = ::refresh + ), loadNextPage = ::getNextPage, onFilterSelected = ::onFilterSelected, onCourseSelected = ::onCourseSelected, - updateContent = ::updateContent, showTopBar = showTopBar, showFilters = showFilters, + showCourseFilter = courseId == null, navigateToEdit = navigateToEdit, ) ) val uiState = _uiState.asStateFlow() - var updateJob: Job? = null init { - removeCourseFilterIfNeeded() - loadCourses() loadData() } - private fun loadCourses(forceNetwork: Boolean = false) { - viewModelScope.launch { - when (val result = repository.getCourses(forceNetwork)) { - is DataResult.Success -> { - _uiState.update { it.copy(courses = result.data) } - } - - is DataResult.Fail -> { - _uiState.update { it.copy(courses = emptyList()) } - } - } + fun loadData() { + loadJob?.cancel() + loadJob = viewModelScope.tryLaunch { + _uiState.update { it.copy(loadingState = it.loadingState.copy(isLoading = true, isError = false, errorMessage = null)) } + fetchData() + _uiState.update { it.copy(loadingState = it.loadingState.copy(isLoading = false)) } + } catch { + _uiState.update { it.copy(loadingState = it.loadingState.copy(isLoading = false, isError = true, errorMessage = "Failed to load notes")) } } } fun refresh() { - updateJob?.cancel() - loadCourses(forceNetwork = true) - loadData(isRefreshing = true) + pageInfo = null + loadJob?.cancel() + loadJob = viewModelScope.tryLaunch { + _uiState.update { it.copy(loadingState = it.loadingState.copy(isRefreshing = true)) } + fetchData(forceNetwork = true) + _uiState.update { it.copy(loadingState = it.loadingState.copy(isRefreshing = false, isError = false, errorMessage = null)) } + } catch { + _uiState.update { it.copy(loadingState = it.loadingState.copy(isRefreshing = false, snackbarMessage = "Failed to load notes")) } + } } - private fun loadData( - after: String? = null, - courseId: Long? = this.courseId, - isLoadingMore: Boolean = false, - isRefreshing: Boolean = false - ) { - updateJob = viewModelScope.tryLaunch { - _uiState.update { - when { - isRefreshing -> it.copy(isRefreshing = true, isError = false) - isLoadingMore -> it.copy(isLoadingMore = true, isError = false) - else -> it.copy(isLoading = true, isError = false) - } - } - - val notesResponse = repository.getNotes( - after = after, - before = null, - filterType = uiState.value.selectedFilter, - courseId = courseId, - objectTypeAndId = objectTypeAndId, - orderDirection = OrderDirection.descending - ) - cursorId = notesResponse.edges?.firstOrNull()?.cursor - pageInfo = notesResponse.pageInfo - - val oldNotes = if (courseId != null) { - uiState.value.notes.filter { it.courseId == this@NotebookViewModel.courseId } - } else { - uiState.value.notes - } - val newNotes = notesResponse.mapToNotes().also { notes -> - // Filter notes by courseId if applicable - if (courseId != null) { - notes.filter { it.courseId == courseId } - } else { - notes - } - } - - _uiState.update { - it.copy( - isError = false, - isLoading = false, - isLoadingMore = false, - isRefreshing = false, - notes = if (isLoadingMore) oldNotes + newNotes else newNotes, - hasNextPage = notesResponse.pageInfo.hasNextPage, - ) - } - } catch { - _uiState.update { - it.copy( - isError = true, - isLoading = false, - isLoadingMore = false, - isRefreshing = false, - hasNextPage = false, - ) - } + private suspend fun fetchData(forceNetwork: Boolean = false) { + fetchCourses(forceNetwork) + val notes = fetchNotes(forceNetwork) + _uiState.update { it.copy(notes = notes) } + } + + private suspend fun fetchCourses(forceNetwork: Boolean = false) { + val courses = repository.getCourses(forceNetwork).dataOrThrow + _uiState.update { it.copy(courses = courses) } + } + + private suspend fun fetchNotes(forceNetwork: Boolean = false): List { + val notesResponse = repository.getNotes( + after = pageInfo?.endCursor, + before = null, + filterType = uiState.value.selectedFilter, + courseId = courseId, + objectTypeAndId = objectTypeAndId, + orderDirection = OrderDirection.descending, + forceNetwork = forceNetwork + ) + pageInfo = notesResponse.pageInfo + + _uiState.update { + it.copy(hasNextPage = notesResponse.pageInfo.hasNextPage,) } + + return notesResponse.mapToNotes() } private fun getNextPage() { - if (!uiState.value.isLoadingMore && uiState.value.hasNextPage) { - loadData(after = pageInfo?.endCursor, isLoadingMore = true) + loadJob = viewModelScope.tryLaunch { + _uiState.update { it.copy(isLoadingMore = true) } + val notes = fetchNotes() + _uiState.update { it.copy(notes = it.notes + notes, isLoadingMore = false) } + } catch { + _uiState.update { it.copy(loadingState = it.loadingState.copy(snackbarMessage = "Failed to load notes")) } } } private fun onFilterSelected(newFilter: NotebookType?) { - _uiState.update { currentState -> - currentState.copy(selectedFilter = newFilter) - } - updateJob?.cancel() + pageInfo = null + _uiState.update { it.copy(selectedFilter = newFilter) } loadData() } private fun onCourseSelected(course: CourseWithProgress?) { - _uiState.update { currentState -> - currentState.copy(selectedCourse = course) - } - updateJob?.cancel() + pageInfo = null + _uiState.update { it.copy(selectedCourse = course) } courseId = course?.courseId - loadData(courseId = courseId) - } - - private fun updateContent(courseId: Long?, objectTypeAndId: Pair?) { - if (courseId != this.courseId || objectTypeAndId != this.objectTypeAndId) { - this.courseId = courseId - this.objectTypeAndId = objectTypeAndId - updateJob?.cancel() - loadData() - } + loadData() } fun updateFilters(courseId: Long? = null, objectTypeAndId: Pair? = null) { if (courseId != this.courseId) { + pageInfo = null this.courseId = courseId this.objectTypeAndId = objectTypeAndId - updateJob?.cancel() - loadData() } + loadData() } - private fun removeCourseFilterIfNeeded() { - if (courseId != null) { - _uiState.update { it.copy(showCourseFilter = false) } - } + private fun onSnackbarDismiss() { + _uiState.update { it.copy(loadingState = it.loadingState.copy(snackbarMessage = null)) } } fun updateScreenState( diff --git a/libs/horizon/src/main/res/values/strings.xml b/libs/horizon/src/main/res/values/strings.xml index c284711157..b26eb410b5 100644 --- a/libs/horizon/src/main/res/values/strings.xml +++ b/libs/horizon/src/main/res/values/strings.xml @@ -279,7 +279,7 @@ Copy Add note Mark important - Mark confusing + Mark unclear Score: %1$s/%2$s Attempts This assignment allows multiple attempts. Once you\'ve made a submission, you can view it here. diff --git a/libs/horizon/src/test/java/com/instructure/horizon/features/notebook/NotebookRepositoryTest.kt b/libs/horizon/src/test/java/com/instructure/horizon/features/notebook/NotebookRepositoryTest.kt index 9780e4169f..564fe716d6 100644 --- a/libs/horizon/src/test/java/com/instructure/horizon/features/notebook/NotebookRepositoryTest.kt +++ b/libs/horizon/src/test/java/com/instructure/horizon/features/notebook/NotebookRepositoryTest.kt @@ -156,16 +156,11 @@ class NotebookRepositoryTest { ), any(), any(), any(), any(), any()) } } - @Test + @Test(expected = Exception::class) fun `Test error handling for notes retrieval`() = runTest { coEvery { redwoodApiManager.getNotes(any(), any(), any(), any(), any(), any()) } throws Exception("Network error") - try { - getRepository().getNotes() - junit.framework.TestCase.fail("Expected exception to be thrown") - } catch (e: Exception) { - assertEquals("Network error", e.message) - } + getRepository().getNotes() } @Test @@ -201,17 +196,6 @@ class NotebookRepositoryTest { coVerify { horizonGetCoursesManager.getCoursesWithProgress(userId = 123L, forceNetwork = false) } } - @Test - fun `Test getCourses handles missing user`() = runTest { - every { apiPrefs.user } returns null - val mockCourses = listOf(mockk(relaxed = true)) - coEvery { horizonGetCoursesManager.getCoursesWithProgress(any(), any()) } returns DataResult.Success(mockCourses) - - getRepository().getCourses() - - coVerify { horizonGetCoursesManager.getCoursesWithProgress(userId = 0L, forceNetwork = false) } - } - private fun getRepository(): NotebookRepository { return NotebookRepository(redwoodApiManager, horizonGetCoursesManager, apiPrefs) } diff --git a/libs/horizon/src/test/java/com/instructure/horizon/features/notebook/NotebookViewModelTest.kt b/libs/horizon/src/test/java/com/instructure/horizon/features/notebook/NotebookViewModelTest.kt index b3e8697f02..3863d4a342 100644 --- a/libs/horizon/src/test/java/com/instructure/horizon/features/notebook/NotebookViewModelTest.kt +++ b/libs/horizon/src/test/java/com/instructure/horizon/features/notebook/NotebookViewModelTest.kt @@ -91,7 +91,7 @@ class NotebookViewModelTest { fun `Test data loads successfully on init`() { val viewModel = getViewModel() - assertFalse(viewModel.uiState.value.isLoading) + assertFalse(viewModel.uiState.value.loadingState.isLoading) coVerify { repository.getNotes(any(), any(), any(), any(), any(), any(), any()) } } @@ -108,7 +108,7 @@ class NotebookViewModelTest { val viewModel = getViewModel() - assertFalse(viewModel.uiState.value.isLoading) + assertFalse(viewModel.uiState.value.loadingState.isLoading) assertTrue(viewModel.uiState.value.notes.isEmpty()) } @@ -217,24 +217,6 @@ class NotebookViewModelTest { assertTrue(viewModel.uiState.value.showTopBar) } - @Test - fun `Test updateContent with different courseId reloads data`() = runTest { - val viewModel = getViewModel() - - viewModel.uiState.value.updateContent(456L, Pair("Assignment", "123")) - - coVerify(atLeast = 1) { repository.getNotes(any(), any(), any(), any(), 456L, Pair("Assignment", "123"), any()) } - } - - @Test - fun `Test updateContent with same courseId does not reload data`() = runTest { - val viewModel = getViewModel() - - viewModel.uiState.value.updateContent(null, null) - - coVerify(exactly = 1) { repository.getNotes(any(), any(), any(), any(), any(), any(), any()) } - } - @Test fun `Test loadNextPage triggers with valid next page`() = runTest { coEvery { repository.getNotes(any(), any(), any(), any(), any(), any(), any()) } returns testNotes.copy( @@ -247,20 +229,6 @@ class NotebookViewModelTest { coVerify(atLeast = 2) { repository.getNotes(any(), any(), any(), any(), any(), any(), any()) } } - @Test - fun `Test loadNextPage does not trigger when no next page`() = runTest { - val notesWithoutNext = testNotes.copy( - pageInfo = testNotes.pageInfo.copy(hasNextPage = false) - ) - coEvery { repository.getNotes(any(), any(), any(), any(), any(), any(), any()) } returns notesWithoutNext - - val viewModel = getViewModel() - - viewModel.uiState.value.loadNextPage() - - coVerify(exactly = 1) { repository.getNotes(any(), any(), any(), any(), any(), any(), any()) } - } - @Test fun `Test course filter is hidden when courseId is present`() = runTest { savedStateHandle["courseId"] = "123" From edb800072b2a8b655b9d15d560c9656a9dd8e2c6 Mon Sep 17 00:00:00 2001 From: domonkosadam Date: Tue, 25 Nov 2025 09:36:00 +0100 Subject: [PATCH 13/33] UI improvements --- .../DashboardAnnouncementBannerViewModel.kt | 2 +- .../DashboardAnnouncementBannerWidget.kt | 2 +- .../features/notebook/NotebookRepository.kt | 5 +-- .../features/notebook/NotebookViewModel.kt | 40 ++++++++++++++----- libs/horizon/src/main/res/values/strings.xml | 1 + 5 files changed, 34 insertions(+), 16 deletions(-) diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/dashboard/widget/announcement/DashboardAnnouncementBannerViewModel.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/dashboard/widget/announcement/DashboardAnnouncementBannerViewModel.kt index 198d5a1de6..24ac157271 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/dashboard/widget/announcement/DashboardAnnouncementBannerViewModel.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/dashboard/widget/announcement/DashboardAnnouncementBannerViewModel.kt @@ -104,7 +104,7 @@ private fun AnnouncementBannerItem.toPaginatedWidgetCardItemState(context: Conte return DashboardPaginatedWidgetCardItemState( headerState = DashboardPaginatedWidgetCardHeaderState( label = context.getString(R.string.notificationsAnnouncementCategoryLabel), - color = HorizonColors.Surface.institution().copy(alpha = 0.1f), + color = HorizonColors.PrimitivesSky.sky12, iconRes = R.drawable.campaign ), source = source, diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/dashboard/widget/announcement/DashboardAnnouncementBannerWidget.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/dashboard/widget/announcement/DashboardAnnouncementBannerWidget.kt index e68278f524..f3dc4e86c8 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/dashboard/widget/announcement/DashboardAnnouncementBannerWidget.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/dashboard/widget/announcement/DashboardAnnouncementBannerWidget.kt @@ -75,7 +75,7 @@ fun DashboardAnnouncementBannerSection( DashboardWidgetCardError( stringResource(R.string.notificationsAnnouncementCategoryLabel), R.drawable.campaign, - HorizonColors.Surface.institution().copy(alpha = 0.1f), + HorizonColors.PrimitivesSky.sky12, false, DashboardWidgetPageState.Empty, { state.onRefresh {} }, diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookRepository.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookRepository.kt index 45c4d5d222..88cb454dac 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookRepository.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookRepository.kt @@ -21,7 +21,6 @@ import com.instructure.canvasapi2.managers.graphql.horizon.CourseWithProgress import com.instructure.canvasapi2.managers.graphql.horizon.HorizonGetCoursesManager import com.instructure.canvasapi2.managers.graphql.horizon.redwood.RedwoodApiManager import com.instructure.canvasapi2.utils.ApiPrefs -import com.instructure.canvasapi2.utils.DataResult import com.instructure.horizon.features.notebook.common.model.NotebookType import com.instructure.redwood.QueryNotesQuery import com.instructure.redwood.type.LearningObjectFilter @@ -89,10 +88,10 @@ class NotebookRepository @Inject constructor( } - suspend fun getCourses(forceNetwork: Boolean = false): DataResult> { + suspend fun getCourses(forceNetwork: Boolean = false): List { return horizonGetCoursesManager.getCoursesWithProgress( userId = apiPrefs.user?.id ?: 0L, forceNetwork = forceNetwork - ) + ).dataOrNull.orEmpty() } } \ No newline at end of file diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookViewModel.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookViewModel.kt index 1cd8ad0876..e52ca48adb 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookViewModel.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookViewModel.kt @@ -16,12 +16,14 @@ */ package com.instructure.horizon.features.notebook +import android.content.Context import androidx.lifecycle.SavedStateHandle import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import com.instructure.canvasapi2.managers.graphql.horizon.CourseWithProgress import com.instructure.canvasapi2.utils.weave.catch import com.instructure.canvasapi2.utils.weave.tryLaunch +import com.instructure.horizon.R import com.instructure.horizon.features.notebook.common.model.Note import com.instructure.horizon.features.notebook.common.model.NotebookType import com.instructure.horizon.features.notebook.common.model.mapToNotes @@ -30,6 +32,7 @@ import com.instructure.horizon.horizonui.platform.LoadingState import com.instructure.redwood.QueryNotesQuery import com.instructure.redwood.type.OrderDirection import dagger.hilt.android.lifecycle.HiltViewModel +import dagger.hilt.android.qualifiers.ApplicationContext import kotlinx.coroutines.Job import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.asStateFlow @@ -38,6 +41,7 @@ import javax.inject.Inject @HiltViewModel class NotebookViewModel @Inject constructor( + @ApplicationContext private val context: Context, private val repository: NotebookRepository, savedStateHandle: SavedStateHandle, ) : ViewModel() { @@ -78,11 +82,19 @@ class NotebookViewModel @Inject constructor( fun loadData() { loadJob?.cancel() loadJob = viewModelScope.tryLaunch { - _uiState.update { it.copy(loadingState = it.loadingState.copy(isLoading = true, isError = false, errorMessage = null)) } + _uiState.update { it.copy(loadingState = it.loadingState.copy(isLoading = true, isRefreshing = false, isError = false, errorMessage = null)) } fetchData() - _uiState.update { it.copy(loadingState = it.loadingState.copy(isLoading = false)) } + _uiState.update { it.copy(loadingState = it.loadingState.copy(isLoading = false, isError = false, errorMessage = null)) } } catch { - _uiState.update { it.copy(loadingState = it.loadingState.copy(isLoading = false, isError = true, errorMessage = "Failed to load notes")) } + _uiState.update { it.copy( + loadingState = it.loadingState.copy( + isLoading = false, + isError = true, + errorMessage = context.getString( + R.string.notebookFailedToLoadErrorMessage + ) + ) + ) } } } @@ -94,7 +106,14 @@ class NotebookViewModel @Inject constructor( fetchData(forceNetwork = true) _uiState.update { it.copy(loadingState = it.loadingState.copy(isRefreshing = false, isError = false, errorMessage = null)) } } catch { - _uiState.update { it.copy(loadingState = it.loadingState.copy(isRefreshing = false, snackbarMessage = "Failed to load notes")) } + _uiState.update { it.copy( + loadingState = it.loadingState.copy( + isRefreshing = false, + snackbarMessage = context.getString( + R.string.notebookFailedToLoadErrorMessage + ) + ) + ) } } } @@ -105,7 +124,7 @@ class NotebookViewModel @Inject constructor( } private suspend fun fetchCourses(forceNetwork: Boolean = false) { - val courses = repository.getCourses(forceNetwork).dataOrThrow + val courses = repository.getCourses(forceNetwork) _uiState.update { it.copy(courses = courses) } } @@ -122,13 +141,14 @@ class NotebookViewModel @Inject constructor( pageInfo = notesResponse.pageInfo _uiState.update { - it.copy(hasNextPage = notesResponse.pageInfo.hasNextPage,) + it.copy(hasNextPage = notesResponse.pageInfo.hasNextPage) } return notesResponse.mapToNotes() } private fun getNextPage() { + loadJob?.cancel() loadJob = viewModelScope.tryLaunch { _uiState.update { it.copy(isLoadingMore = true) } val notes = fetchNotes() @@ -152,11 +172,9 @@ class NotebookViewModel @Inject constructor( } fun updateFilters(courseId: Long? = null, objectTypeAndId: Pair? = null) { - if (courseId != this.courseId) { - pageInfo = null - this.courseId = courseId - this.objectTypeAndId = objectTypeAndId - } + pageInfo = null + this.courseId = courseId + this.objectTypeAndId = objectTypeAndId loadData() } diff --git a/libs/horizon/src/main/res/values/strings.xml b/libs/horizon/src/main/res/values/strings.xml index b26eb410b5..4c6155b6ee 100644 --- a/libs/horizon/src/main/res/values/strings.xml +++ b/libs/horizon/src/main/res/values/strings.xml @@ -450,4 +450,5 @@ Adjust your filters or create a new note to get started. Failed to load Notebook Please try again. + Failed to load notes \ No newline at end of file From 230729d9ca9789cafcc6c544bc749e62efa46b45 Mon Sep 17 00:00:00 2001 From: domonkosadam Date: Tue, 25 Nov 2025 10:14:48 +0100 Subject: [PATCH 14/33] Implement dropdown icons --- .../features/notebook/NotebookScreen.kt | 46 +----- .../notebook/addedit/AddEditNoteScreen.kt | 24 +-- .../common/composable/NotebookTypeSelect.kt | 153 ++++++++---------- .../notebook/common/model/NotebookType.kt | 2 +- .../horizonui/molecules/DropdownChip.kt | 11 ++ libs/horizon/src/main/res/values/strings.xml | 2 +- 6 files changed, 86 insertions(+), 152 deletions(-) diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt index 5fde016c38..5b4f26ba1a 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt @@ -41,7 +41,6 @@ import androidx.compose.runtime.remember import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip -import androidx.compose.ui.graphics.Color import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.res.stringResource import androidx.compose.ui.text.style.TextOverflow @@ -55,9 +54,9 @@ import com.instructure.horizon.R import com.instructure.horizon.features.notebook.common.composable.NotebookAppBar import com.instructure.horizon.features.notebook.common.composable.NotebookHighlightedText import com.instructure.horizon.features.notebook.common.composable.NotebookPill +import com.instructure.horizon.features.notebook.common.composable.NotebookTypeSelect import com.instructure.horizon.features.notebook.common.composable.toNotebookLocalisedDateFormat import com.instructure.horizon.features.notebook.common.model.Note -import com.instructure.horizon.features.notebook.common.model.NotebookType import com.instructure.horizon.features.notebook.navigation.NotebookRoute import com.instructure.horizon.horizonui.foundation.HorizonColors import com.instructure.horizon.horizonui.foundation.HorizonCornerRadius @@ -213,37 +212,6 @@ private fun FilterContent( ) { val context = LocalContext.current val defaultBackgroundColor = HorizonColors.PrimitivesGrey.grey12() - val importantBgColor = HorizonColors.PrimitivesSea.sea12() - val confusingBgColor = HorizonColors.PrimitivesRed.red12() - - // Type filter items - val allNotesItem = DropdownItem( - value = null as NotebookType?, - label = context.getString(R.string.notebookTypeAllNotes), - iconRes = R.drawable.menu, - iconTint = HorizonColors.Icon.default(), - backgroundColor = defaultBackgroundColor - ) - - val typeItems = remember { - listOf( - allNotesItem, - DropdownItem( - value = NotebookType.Important, - label = context.getString(NotebookType.Important.labelRes), - iconRes = NotebookType.Important.iconRes, - iconTint = Color(context.getColor(NotebookType.Important.color)), - backgroundColor = importantBgColor - ), - DropdownItem( - value = NotebookType.Confusing, - label = context.getString(NotebookType.Confusing.labelRes), - iconRes = NotebookType.Confusing.iconRes, - iconTint = Color(context.getColor(NotebookType.Confusing.color)), - backgroundColor = confusingBgColor - ) - ) - } // Course filter items val allCoursesItem = DropdownItem( @@ -266,9 +234,6 @@ private fun FilterContent( } } - val selectedTypeItem = - if (state.selectedFilter == null) allNotesItem else typeItems.find { it.value == state.selectedFilter } - val selectedCourseItem = if (state.selectedCourse == null) allCoursesItem else courseItems.find { it.value == state.selectedCourse } @@ -293,14 +258,7 @@ private fun FilterContent( } if (state.showNoteTypeFilter) { - DropdownChip( - items = typeItems, - selectedItem = selectedTypeItem, - onItemSelected = { item -> state.onFilterSelected(item?.value) }, - placeholder = stringResource(R.string.notebookFilterTypePlaceholder), - dropdownWidth = 178.dp, - verticalPadding = 6.dp - ) + NotebookTypeSelect(state.selectedFilter, state.onFilterSelected, false, true) } } diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/addedit/AddEditNoteScreen.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/addedit/AddEditNoteScreen.kt index e4a575c7ad..9675d870f5 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/addedit/AddEditNoteScreen.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/addedit/AddEditNoteScreen.kt @@ -19,13 +19,11 @@ package com.instructure.horizon.features.notebook.addedit import androidx.activity.compose.BackHandler import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column -import androidx.compose.foundation.layout.IntrinsicSize import androidx.compose.foundation.layout.PaddingValues import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding -import androidx.compose.foundation.layout.width import androidx.compose.foundation.rememberScrollState import androidx.compose.foundation.verticalScroll import androidx.compose.material3.CenterAlignedTopAppBar @@ -35,10 +33,6 @@ import androidx.compose.material3.Text import androidx.compose.material3.TopAppBarDefaults import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect -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.platform.LocalContext @@ -55,6 +49,7 @@ import com.instructure.canvasapi2.managers.graphql.horizon.redwood.NoteHighlight import com.instructure.canvasapi2.utils.ContextKeeper import com.instructure.horizon.R import com.instructure.horizon.features.notebook.common.composable.NotebookHighlightedText +import com.instructure.horizon.features.notebook.common.composable.NotebookTypeSelect import com.instructure.horizon.features.notebook.common.model.NotebookType import com.instructure.horizon.horizonui.foundation.HorizonColors import com.instructure.horizon.horizonui.foundation.HorizonSpace @@ -69,9 +64,6 @@ import com.instructure.horizon.horizonui.molecules.Spinner import com.instructure.horizon.horizonui.organisms.Modal import com.instructure.horizon.horizonui.organisms.ModalDialogState import com.instructure.horizon.horizonui.organisms.inputs.common.InputLabelRequired -import com.instructure.horizon.horizonui.organisms.inputs.singleselect.SingleSelect -import com.instructure.horizon.horizonui.organisms.inputs.singleselect.SingleSelectInputSize -import com.instructure.horizon.horizonui.organisms.inputs.singleselect.SingleSelectState import com.instructure.horizon.horizonui.organisms.inputs.textarea.TextArea import com.instructure.horizon.horizonui.organisms.inputs.textarea.TextAreaState import com.instructure.pandautils.utils.ViewStyler @@ -173,19 +165,7 @@ private fun AddEditNoteContent(state: AddEditNoteUiState, padding: PaddingValues ) { HorizonSpace(SpaceSize.SPACE_24) - var isMenuOpen by remember { mutableStateOf(false) } - SingleSelect( - SingleSelectState( - options = NotebookType.entries.map { it.name }, - selectedOption = state.type?.name, - size = SingleSelectInputSize.Small, - onOptionSelected = { state.onTypeChanged(NotebookType.valueOf(it)) }, - isMenuOpen = isMenuOpen, - onMenuOpenChanged = { isMenuOpen = it }, - isFullWidth = false, - ), - modifier = Modifier.width(IntrinsicSize.Min) - ) + NotebookTypeSelect(state.type, state.onTypeChanged, true, false) HorizonSpace(SpaceSize.SPACE_24) diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/common/composable/NotebookTypeSelect.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/common/composable/NotebookTypeSelect.kt index c41d9749e7..5fd78f56dc 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/common/composable/NotebookTypeSelect.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/common/composable/NotebookTypeSelect.kt @@ -16,119 +16,104 @@ */ package com.instructure.horizon.features.notebook.common.composable -import androidx.compose.foundation.background -import androidx.compose.foundation.border -import androidx.compose.foundation.clickable -import androidx.compose.foundation.layout.Column -import androidx.compose.foundation.layout.padding -import androidx.compose.foundation.layout.size -import androidx.compose.material3.Icon -import androidx.compose.material3.Text import androidx.compose.runtime.Composable -import androidx.compose.ui.Alignment +import androidx.compose.runtime.remember import androidx.compose.ui.Modifier -import androidx.compose.ui.draw.clip import androidx.compose.ui.graphics.Color import androidx.compose.ui.platform.LocalContext -import androidx.compose.ui.res.colorResource -import androidx.compose.ui.res.painterResource import androidx.compose.ui.res.stringResource import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.unit.dp -import com.instructure.canvasapi2.utils.ContextKeeper +import com.instructure.horizon.R import com.instructure.horizon.features.notebook.common.model.NotebookType -import com.instructure.horizon.horizonui.foundation.HorizonBorder import com.instructure.horizon.horizonui.foundation.HorizonColors -import com.instructure.horizon.horizonui.foundation.HorizonCornerRadius -import com.instructure.horizon.horizonui.foundation.HorizonSpace -import com.instructure.horizon.horizonui.foundation.HorizonTypography -import com.instructure.horizon.horizonui.foundation.SpaceSize +import com.instructure.horizon.horizonui.molecules.DropdownChip +import com.instructure.horizon.horizonui.molecules.DropdownItem @Composable fun NotebookTypeSelect( - type: NotebookType, - isSelected: Boolean, - onSelect: () -> Unit, - modifier: Modifier = Modifier + selected: NotebookType?, + onSelect: (NotebookType?) -> Unit, + showIcons: Boolean, + showAllOption: Boolean, + modifier: Modifier = Modifier, ) { - val borderColor = if (isSelected) colorResource(type.color) else HorizonColors.LineAndBorder.containerStroke() - val iconColor = if (isSelected) colorResource(type.color) else HorizonColors.Icon.default() - val textColor = if (isSelected) colorResource(type.color) else HorizonColors.Text.body() - Column( - horizontalAlignment = Alignment.CenterHorizontally, - modifier = modifier - .clip(HorizonCornerRadius.level2) - .border( - HorizonBorder.level1(color = borderColor), - HorizonCornerRadius.level2 + val context = LocalContext.current + val defaultBackgroundColor = HorizonColors.PrimitivesGrey.grey12() + val importantBgColor = HorizonColors.PrimitivesSea.sea12() + val confusingBgColor = HorizonColors.PrimitivesRed.red12() + val allNotesItem = DropdownItem( + value = null as NotebookType?, + label = context.getString(R.string.notebookTypeAllNotes), + iconRes = R.drawable.menu, + iconTint = HorizonColors.Icon.default(), + backgroundColor = defaultBackgroundColor + ) + val typeItems = remember { + buildList { + if (showAllOption) { + add(allNotesItem) + } + add( + DropdownItem( + value = NotebookType.Important, + label = context.getString(NotebookType.Important.labelRes), + iconRes = NotebookType.Important.iconRes, + iconTint = Color(context.getColor(NotebookType.Important.color)), + backgroundColor = importantBgColor + ) ) - .background(Color.White) - .clickable { - onSelect() - }, - ) { - Column( - horizontalAlignment = Alignment.CenterHorizontally, - modifier = Modifier.padding(24.dp) - ) { - Icon( - painter = painterResource(type.iconRes), - contentDescription = null, - tint = iconColor, - modifier = Modifier.size(24.dp) - ) - - HorizonSpace(SpaceSize.SPACE_8) - - Text( - text = stringResource(type.labelRes), - style = HorizonTypography.buttonTextLarge, - color = textColor, + add( + DropdownItem( + value = NotebookType.Confusing, + label = context.getString(NotebookType.Confusing.labelRes), + iconRes = NotebookType.Confusing.iconRes, + iconTint = Color(context.getColor(NotebookType.Confusing.color)), + backgroundColor = confusingBgColor + ) ) } } -} + val selectedTypeItem = + if (selected == null) allNotesItem else typeItems.find { it.value == selected } -@Composable -@Preview -private fun NotebookTypeSelectConfusingSelectedPreview() { - ContextKeeper.appContext = LocalContext.current - NotebookTypeSelect( - type = NotebookType.Confusing, - isSelected = true, - onSelect = {} + + DropdownChip( + items = typeItems, + selectedItem = selectedTypeItem, + onItemSelected = { item -> onSelect(item?.value) }, + placeholder = stringResource(R.string.notebookFilterTypePlaceholder), + dropdownWidth = 178.dp, + verticalPadding = 6.dp, + showIconCollapsed = showIcons, + borderColor = if (showIcons) { + selectedTypeItem?.iconTint ?: HorizonColors.LineAndBorder.lineStroke() + } else { + HorizonColors.LineAndBorder.lineStroke() + }, + contentColor = if (showIcons) { + selectedTypeItem?.iconTint ?: HorizonColors.Text.body() + } else { + HorizonColors.Text.body() + }, + modifier = modifier ) } @Composable @Preview -private fun NotebookTypeSelectConfusingNotSelectedPreview() { - ContextKeeper.appContext = LocalContext.current - NotebookTypeSelect( - type = NotebookType.Confusing, - isSelected = false, - onSelect = {} - ) +private fun NotebookTypeSelectAllPreview() { + NotebookTypeSelect(null, {}, true, true) } @Composable @Preview -private fun NotebookTypeSelectImportantSelectedPreview() { - ContextKeeper.appContext = LocalContext.current - NotebookTypeSelect( - type = NotebookType.Important, - isSelected = true, - onSelect = {} - ) +private fun NotebookTypeSelectImportantPreview() { + NotebookTypeSelect(NotebookType.Important, {}, true, true) } @Composable @Preview -private fun NotebookTypeSelectImportantNotSelectedPreview() { - ContextKeeper.appContext = LocalContext.current - NotebookTypeSelect( - type = NotebookType.Important, - isSelected = false, - onSelect = {} - ) +private fun NotebookTypeSelectConfusingPreview() { + NotebookTypeSelect(NotebookType.Confusing, {}, true, true) } \ No newline at end of file diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/common/model/NotebookType.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/common/model/NotebookType.kt index ba0a59818a..73cecec714 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/common/model/NotebookType.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/common/model/NotebookType.kt @@ -10,6 +10,6 @@ enum class NotebookType( @DrawableRes val iconRes: Int, @ColorRes val color: Int ) { - Confusing(R.string.notebookTypeConfusing, R.drawable.help, R.color.icon_error), + Confusing(R.string.notebookTypeUnclear, R.drawable.help, R.color.icon_error), Important(R.string.notebookTypeImportant, R.drawable.keep_pin, R.color.icon_action), } diff --git a/libs/horizon/src/main/java/com/instructure/horizon/horizonui/molecules/DropdownChip.kt b/libs/horizon/src/main/java/com/instructure/horizon/horizonui/molecules/DropdownChip.kt index 66e3dcdf65..feae7094cf 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/horizonui/molecules/DropdownChip.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/horizonui/molecules/DropdownChip.kt @@ -75,6 +75,7 @@ fun DropdownChip( modifier: Modifier = Modifier, dropdownWidth: Dp? = null, placeholder: String, + showIconCollapsed: Boolean = false, borderColor: Color = HorizonColors.LineAndBorder.lineStroke(), contentColor: Color = HorizonColors.Text.body(), verticalPadding: Dp = 0.dp @@ -121,6 +122,16 @@ fun DropdownChip( contentDescription = selectedItem?.label ?: placeholder } ) { + if (showIconCollapsed && selectedItem?.iconRes != null) { + Icon( + painter = painterResource(selectedItem.iconRes), + contentDescription = null, + modifier = Modifier + .size(24.dp) + .padding(2.dp), + tint = selectedItem.iconTint ?: HorizonColors.Icon.default() + ) + } Text( text = selectedItem?.label ?: placeholder, style = HorizonTypography.p2, diff --git a/libs/horizon/src/main/res/values/strings.xml b/libs/horizon/src/main/res/values/strings.xml index 2b7c4fb282..8341112cbd 100644 --- a/libs/horizon/src/main/res/values/strings.xml +++ b/libs/horizon/src/main/res/values/strings.xml @@ -247,7 +247,7 @@ Once you submit this attempt, you won’t be able to make any changes. Cancel Submit attempt - Unclear + Unclear Important All notes Notebook From 4a7696f37703f7f154037b49b08a74cbba3266cc Mon Sep 17 00:00:00 2001 From: domonkosadam Date: Tue, 25 Nov 2025 10:45:07 +0100 Subject: [PATCH 15/33] Fix tests --- .../fakes/FakeGetHorizonCourseManager.kt | 44 +++++++++++-------- .../mockcanvas/fakes/FakeRedwoodApiManager.kt | 3 +- .../notebook/NotebookRepositoryTest.kt | 4 +- .../notebook/NotebookViewModelTest.kt | 11 ++--- 4 files changed, 34 insertions(+), 28 deletions(-) diff --git a/automation/espresso/src/main/kotlin/com/instructure/canvas/espresso/mockcanvas/fakes/FakeGetHorizonCourseManager.kt b/automation/espresso/src/main/kotlin/com/instructure/canvas/espresso/mockcanvas/fakes/FakeGetHorizonCourseManager.kt index 207a909deb..f8fa0f968f 100644 --- a/automation/espresso/src/main/kotlin/com/instructure/canvas/espresso/mockcanvas/fakes/FakeGetHorizonCourseManager.kt +++ b/automation/espresso/src/main/kotlin/com/instructure/canvas/espresso/mockcanvas/fakes/FakeGetHorizonCourseManager.kt @@ -90,25 +90,31 @@ class FakeGetHorizonCourseManager(): HorizonGetCoursesManager { fun getCourses(): List { val courses = MockCanvas.data.courses.values.toList() - val activeCourse = CourseWithProgress( - courseId = courses[0].id, - courseName = courses[0].name, - courseSyllabus = "Syllabus for Course 1", - progress = 0.25 - ) - val completedCourse = CourseWithProgress( - courseId = courses[1].id, - courseName = courses[1].name, - courseSyllabus = "Syllabus for Course 2", - progress = 1.0 - ) - val invitedCourse = CourseWithProgress( - courseId = courses[2].id, - courseName = courses[2].name, - courseSyllabus = null, - progress = 0.0 - ) + val activeCourse = if (courses.size > 0) { + CourseWithProgress( + courseId = courses[0].id, + courseName = courses[0].name, + courseSyllabus = "Syllabus for Course 1", + progress = 0.25 + ) + } else { null } + val completedCourse = if (courses.size > 1) { + CourseWithProgress( + courseId = courses[1].id, + courseName = courses[1].name, + courseSyllabus = "Syllabus for Course 2", + progress = 1.0 + ) + } else { null } + val invitedCourse = if (courses.size > 2) { + CourseWithProgress( + courseId = courses[2].id, + courseName = courses[2].name, + courseSyllabus = null, + progress = 0.0 + ) + } else { null } - return listOf(activeCourse, completedCourse, invitedCourse) + return listOfNotNull(activeCourse, completedCourse, invitedCourse) } } \ No newline at end of file diff --git a/automation/espresso/src/main/kotlin/com/instructure/canvas/espresso/mockcanvas/fakes/FakeRedwoodApiManager.kt b/automation/espresso/src/main/kotlin/com/instructure/canvas/espresso/mockcanvas/fakes/FakeRedwoodApiManager.kt index 6fe13ff9bf..dd6d253a4f 100644 --- a/automation/espresso/src/main/kotlin/com/instructure/canvas/espresso/mockcanvas/fakes/FakeRedwoodApiManager.kt +++ b/automation/espresso/src/main/kotlin/com/instructure/canvas/espresso/mockcanvas/fakes/FakeRedwoodApiManager.kt @@ -33,7 +33,8 @@ class FakeRedwoodApiManager : RedwoodApiManager { lastN: Int?, after: String?, before: String?, - orderBy: OrderByInput? + orderBy: OrderByInput?, + forceNetwork: Boolean ): QueryNotesQuery.Notes { val edges = notes.map { note -> QueryNotesQuery.Edge( diff --git a/libs/horizon/src/test/java/com/instructure/horizon/features/notebook/NotebookRepositoryTest.kt b/libs/horizon/src/test/java/com/instructure/horizon/features/notebook/NotebookRepositoryTest.kt index 564fe716d6..e73204657d 100644 --- a/libs/horizon/src/test/java/com/instructure/horizon/features/notebook/NotebookRepositoryTest.kt +++ b/libs/horizon/src/test/java/com/instructure/horizon/features/notebook/NotebookRepositoryTest.kt @@ -34,7 +34,6 @@ import io.mockk.every import io.mockk.mockk import junit.framework.TestCase.assertEquals import junit.framework.TestCase.assertNotNull -import junit.framework.TestCase.assertTrue import kotlinx.coroutines.test.runTest import org.junit.Before import org.junit.Test @@ -172,8 +171,7 @@ class NotebookRepositoryTest { val result = getRepository().getCourses() - assertTrue(result is DataResult.Success) - assertEquals(mockCourses, (result as DataResult.Success).data) + assertEquals(mockCourses, result) } @Test diff --git a/libs/horizon/src/test/java/com/instructure/horizon/features/notebook/NotebookViewModelTest.kt b/libs/horizon/src/test/java/com/instructure/horizon/features/notebook/NotebookViewModelTest.kt index 3863d4a342..6d4f3e107e 100644 --- a/libs/horizon/src/test/java/com/instructure/horizon/features/notebook/NotebookViewModelTest.kt +++ b/libs/horizon/src/test/java/com/instructure/horizon/features/notebook/NotebookViewModelTest.kt @@ -16,9 +16,9 @@ */ package com.instructure.horizon.features.notebook +import android.content.Context import androidx.lifecycle.SavedStateHandle import com.instructure.canvasapi2.managers.graphql.horizon.CourseWithProgress -import com.instructure.canvasapi2.utils.DataResult import com.instructure.horizon.features.notebook.common.model.NotebookType import com.instructure.redwood.QueryNotesQuery import io.mockk.coEvery @@ -43,6 +43,7 @@ import java.util.Date @OptIn(ExperimentalCoroutinesApi::class) class NotebookViewModelTest { + private val context: Context = mockk(relaxed = true) private val repository: NotebookRepository = mockk(relaxed = true) private val savedStateHandle = SavedStateHandle() private val testDispatcher = UnconfinedTestDispatcher() @@ -78,7 +79,7 @@ class NotebookViewModelTest { fun setup() { Dispatchers.setMain(testDispatcher) coEvery { repository.getNotes(any(), any(), any(), any(), any(), any(), any()) } returns testNotes - coEvery { repository.getCourses(any()) } returns DataResult.Success(emptyList()) + coEvery { repository.getCourses(any()) } returns emptyList() } @After @@ -164,7 +165,7 @@ class NotebookViewModelTest { mockk(relaxed = true), mockk(relaxed = true) ) - coEvery { repository.getCourses(any()) } returns DataResult.Success(mockCourses) + coEvery { repository.getCourses(any()) } returns mockCourses val viewModel = getViewModel() @@ -173,7 +174,7 @@ class NotebookViewModelTest { @Test fun `Test loadCourses failure sets empty courses`() = runTest { - coEvery { repository.getCourses(any()) } returns DataResult.Fail() + coEvery { repository.getCourses(any()) } returns emptyList() val viewModel = getViewModel() @@ -239,6 +240,6 @@ class NotebookViewModelTest { } private fun getViewModel(): NotebookViewModel { - return NotebookViewModel(repository, savedStateHandle) + return NotebookViewModel(context, repository, savedStateHandle) } } From e6542fa5c6fe11f8041d616f54780d08bd631450 Mon Sep 17 00:00:00 2001 From: domonkosadam Date: Tue, 25 Nov 2025 12:25:43 +0100 Subject: [PATCH 16/33] Fix test --- .../horizon/ui/features/notebook/AddEditNoteScreenUiTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/horizon/src/androidTest/java/com/instructure/horizon/ui/features/notebook/AddEditNoteScreenUiTest.kt b/libs/horizon/src/androidTest/java/com/instructure/horizon/ui/features/notebook/AddEditNoteScreenUiTest.kt index 46c25d4ae5..f78471d0b9 100644 --- a/libs/horizon/src/androidTest/java/com/instructure/horizon/ui/features/notebook/AddEditNoteScreenUiTest.kt +++ b/libs/horizon/src/androidTest/java/com/instructure/horizon/ui/features/notebook/AddEditNoteScreenUiTest.kt @@ -246,7 +246,7 @@ class AddEditNoteScreenUiTest { AddEditNoteScreen(navController, state) { _, _ -> } } - composeTestRule.onNodeWithText("Confusing", useUnmergedTree = true) + composeTestRule.onNodeWithText("Unclear", useUnmergedTree = true) .assertIsDisplayed() } From 322aafdcfe43345f89a0fa425c610706ea555c3e Mon Sep 17 00:00:00 2001 From: domonkosadam Date: Tue, 25 Nov 2025 14:57:40 +0100 Subject: [PATCH 17/33] Implement custom border --- .../features/notebook/NotebookScreen.kt | 10 +- .../horizonui/foundation/HorizonBorder.kt | 103 +++++++++++++++++- 2 files changed, 105 insertions(+), 8 deletions(-) diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt index 5b4f26ba1a..1f750b84ef 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt @@ -42,6 +42,7 @@ import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip import androidx.compose.ui.platform.LocalContext +import androidx.compose.ui.res.colorResource import androidx.compose.ui.res.stringResource import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.tooling.preview.Preview @@ -60,11 +61,10 @@ import com.instructure.horizon.features.notebook.common.model.Note import com.instructure.horizon.features.notebook.navigation.NotebookRoute import com.instructure.horizon.horizonui.foundation.HorizonColors import com.instructure.horizon.horizonui.foundation.HorizonCornerRadius -import com.instructure.horizon.horizonui.foundation.HorizonElevation import com.instructure.horizon.horizonui.foundation.HorizonSpace import com.instructure.horizon.horizonui.foundation.HorizonTypography import com.instructure.horizon.horizonui.foundation.SpaceSize -import com.instructure.horizon.horizonui.foundation.horizonShadow +import com.instructure.horizon.horizonui.foundation.horizonBorder import com.instructure.horizon.horizonui.molecules.Button import com.instructure.horizon.horizonui.molecules.ButtonColor import com.instructure.horizon.horizonui.molecules.ButtonHeight @@ -289,11 +289,7 @@ private fun NoteContent( Column( modifier = Modifier .fillMaxWidth() - .horizonShadow( - elevation = HorizonElevation.level4, - shape = HorizonCornerRadius.level2, - clip = true - ) + .horizonBorder(colorResource(note.type.color), 6.dp, 1.dp, 1.dp, 6.dp, 16.dp) .background( color = HorizonColors.PrimitivesWhite.white10(), shape = HorizonCornerRadius.level2, diff --git a/libs/horizon/src/main/java/com/instructure/horizon/horizonui/foundation/HorizonBorder.kt b/libs/horizon/src/main/java/com/instructure/horizon/horizonui/foundation/HorizonBorder.kt index e5504ee9bd..f7a3315034 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/horizonui/foundation/HorizonBorder.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/horizonui/foundation/HorizonBorder.kt @@ -1,10 +1,111 @@ package com.instructure.horizon.horizonui.foundation import androidx.compose.foundation.BorderStroke +import androidx.compose.runtime.Composable +import androidx.compose.ui.Modifier +import androidx.compose.ui.draw.drawBehind +import androidx.compose.ui.geometry.CornerRadius +import androidx.compose.ui.geometry.Offset +import androidx.compose.ui.geometry.Size +import androidx.compose.ui.graphics.Brush import androidx.compose.ui.graphics.Color +import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp object HorizonBorder { fun level1(color: Color = HorizonColors.LineAndBorder.lineStroke()) = BorderStroke(1.dp, color) fun level2(color: Color = HorizonColors.LineAndBorder.lineStroke()) = BorderStroke(2.dp, color) -} \ No newline at end of file +} + +@Composable +fun Modifier.horizonBorder( + color: Color, + start: Dp, + top: Dp, + end: Dp, + bottom: Dp, + cornerRadius: Dp, +): Modifier { + return drawBehind { + drawRoundRect( + color = color, + topLeft = Offset( + x = -start.toPx(), + y = -top.toPx() + ), + size = Size( + width = size.width + start.toPx() + end.toPx(), + height = size.height + top.toPx() + bottom.toPx() + ), + cornerRadius = CornerRadius(cornerRadius.toPx(), cornerRadius.toPx()), + ) + } +} + +@Composable +fun Modifier.horizonBorderShadow( + color: Color, + start: Dp, + top: Dp, + end: Dp, + bottom: Dp, + cornerRadius: Dp, +): Modifier { + return drawBehind { + drawRoundRect( + brush = Brush.radialGradient( + 0f to color, + 0.9f to color, + 1.0f to color.copy(0.5f), + ), + topLeft = Offset( + x = -start.toPx(), + y = -top.toPx() + ), + size = Size( + width = size.width + start.toPx() + end.toPx(), + height = size.height + top.toPx() + bottom.toPx() + ), + cornerRadius = CornerRadius(cornerRadius.toPx(), cornerRadius.toPx()), + ) + } +} + +@Composable +fun Modifier.horizonBorder( + color: Color, + horizontal: Dp, + vertical: Dp, + cornerRadius: Dp, +): Modifier { + return horizonBorder(color, horizontal, vertical, horizontal, vertical, cornerRadius) +} + +@Composable +fun Modifier.horizonBorder( + color: Color, + all: Dp, + cornerRadius: Dp, +): Modifier { + return horizonBorder(color, all, all, cornerRadius) +} + +@Composable +fun Modifier.horizonBorderShadow( + color: Color, + horizontal: Dp, + vertical: Dp, + cornerRadius: Dp, +): Modifier { + return horizonBorderShadow(color, horizontal, vertical, horizontal, vertical, cornerRadius) +} + +@Composable +fun Modifier.horizonBorderShadow( + color: Color, + all: Dp, + cornerRadius: Dp, +): Modifier { + return horizonBorderShadow(color, all, all, cornerRadius) +} + From 7dd36a0d2f2194aa3d43dcf8205241d81f910a5b Mon Sep 17 00:00:00 2001 From: domonkosadam Date: Tue, 25 Nov 2025 16:00:08 +0100 Subject: [PATCH 18/33] Use custom shadow --- .../features/notebook/NotebookScreen.kt | 4 +- .../horizonui/foundation/HorizonBorder.kt | 47 ++++++++++++------- 2 files changed, 31 insertions(+), 20 deletions(-) diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt index 1f750b84ef..b796b7603b 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt @@ -64,7 +64,7 @@ import com.instructure.horizon.horizonui.foundation.HorizonCornerRadius import com.instructure.horizon.horizonui.foundation.HorizonSpace import com.instructure.horizon.horizonui.foundation.HorizonTypography import com.instructure.horizon.horizonui.foundation.SpaceSize -import com.instructure.horizon.horizonui.foundation.horizonBorder +import com.instructure.horizon.horizonui.foundation.horizonBorderShadow import com.instructure.horizon.horizonui.molecules.Button import com.instructure.horizon.horizonui.molecules.ButtonColor import com.instructure.horizon.horizonui.molecules.ButtonHeight @@ -289,7 +289,7 @@ private fun NoteContent( Column( modifier = Modifier .fillMaxWidth() - .horizonBorder(colorResource(note.type.color), 6.dp, 1.dp, 1.dp, 6.dp, 16.dp) + .horizonBorderShadow(colorResource(note.type.color), 12.dp, 0.dp, 0.dp, 12.dp, 16.dp) .background( color = HorizonColors.PrimitivesWhite.white10(), shape = HorizonCornerRadius.level2, diff --git a/libs/horizon/src/main/java/com/instructure/horizon/horizonui/foundation/HorizonBorder.kt b/libs/horizon/src/main/java/com/instructure/horizon/horizonui/foundation/HorizonBorder.kt index f7a3315034..f77c96fda1 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/horizonui/foundation/HorizonBorder.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/horizonui/foundation/HorizonBorder.kt @@ -1,14 +1,16 @@ package com.instructure.horizon.horizonui.foundation import androidx.compose.foundation.BorderStroke +import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.runtime.Composable import androidx.compose.ui.Modifier import androidx.compose.ui.draw.drawBehind +import androidx.compose.ui.draw.shadow import androidx.compose.ui.geometry.CornerRadius import androidx.compose.ui.geometry.Offset import androidx.compose.ui.geometry.Size -import androidx.compose.ui.graphics.Brush import androidx.compose.ui.graphics.Color +import androidx.compose.ui.layout.layout import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp @@ -51,24 +53,33 @@ fun Modifier.horizonBorderShadow( bottom: Dp, cornerRadius: Dp, ): Modifier { - return drawBehind { - drawRoundRect( - brush = Brush.radialGradient( - 0f to color, - 0.9f to color, - 1.0f to color.copy(0.5f), - ), - topLeft = Offset( - x = -start.toPx(), - y = -top.toPx() - ), - size = Size( - width = size.width + start.toPx() + end.toPx(), - height = size.height + top.toPx() + bottom.toPx() - ), - cornerRadius = CornerRadius(cornerRadius.toPx(), cornerRadius.toPx()), + val maxShadow = maxOf(start, top, end, bottom) + + return this + .layout { measurable, constraints -> + val placeable = measurable.measure(constraints) + val width = placeable.width + start.roundToPx() + end.roundToPx() + val height = placeable.height + top.roundToPx() + bottom.roundToPx() + + layout(width, height) { + placeable.place(start.roundToPx(), top.roundToPx()) + } + } + .shadow( + elevation = maxShadow, + shape = RoundedCornerShape(cornerRadius), + ambientColor = color, + spotColor = color ) - } + .layout { measurable, constraints -> + val placeable = measurable.measure(constraints) + val width = placeable.width - start.roundToPx() - end.roundToPx() + val height = placeable.height - top.roundToPx() - bottom.roundToPx() + + layout(width, height) { + placeable.place(-start.roundToPx(), -top.roundToPx()) + } + } } @Composable From 9daf837e78e6cce2e480a279beff6742a8d7d110 Mon Sep 17 00:00:00 2001 From: domonkosadam Date: Tue, 25 Nov 2025 17:25:49 +0100 Subject: [PATCH 19/33] Fix shadows --- .../features/notebook/NotebookScreen.kt | 1 + .../horizonui/foundation/HorizonBorder.kt | 26 +++++++++---------- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt index b796b7603b..117b22c76c 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt @@ -124,6 +124,7 @@ fun NotebookScreen( } LazyColumn( + verticalArrangement = Arrangement.spacedBy(16.dp), state = scrollState, modifier = Modifier .fillMaxHeight() diff --git a/libs/horizon/src/main/java/com/instructure/horizon/horizonui/foundation/HorizonBorder.kt b/libs/horizon/src/main/java/com/instructure/horizon/horizonui/foundation/HorizonBorder.kt index f77c96fda1..08eb53ee3b 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/horizonui/foundation/HorizonBorder.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/horizonui/foundation/HorizonBorder.kt @@ -1,11 +1,12 @@ package com.instructure.horizon.horizonui.foundation import androidx.compose.foundation.BorderStroke +import androidx.compose.foundation.layout.padding import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.runtime.Composable import androidx.compose.ui.Modifier import androidx.compose.ui.draw.drawBehind -import androidx.compose.ui.draw.shadow +import androidx.compose.ui.draw.dropShadow import androidx.compose.ui.geometry.CornerRadius import androidx.compose.ui.geometry.Offset import androidx.compose.ui.geometry.Size @@ -56,28 +57,27 @@ fun Modifier.horizonBorderShadow( val maxShadow = maxOf(start, top, end, bottom) return this + .padding(start, top, end, bottom) .layout { measurable, constraints -> val placeable = measurable.measure(constraints) - val width = placeable.width + start.roundToPx() + end.roundToPx() - val height = placeable.height + top.roundToPx() + bottom.roundToPx() + val width = placeable.width - start.roundToPx() - end.roundToPx() + val height = placeable.height - top.roundToPx() - bottom.roundToPx() layout(width, height) { - placeable.place(start.roundToPx(), top.roundToPx()) + placeable.place(-start.roundToPx(), -top.roundToPx()) } } - .shadow( - elevation = maxShadow, - shape = RoundedCornerShape(cornerRadius), - ambientColor = color, - spotColor = color - ) + .dropShadow(RoundedCornerShape(cornerRadius)) { + this.color = color.copy(0.1f) + this.radius = maxShadow.toPx() / 2 + } .layout { measurable, constraints -> val placeable = measurable.measure(constraints) - val width = placeable.width - start.roundToPx() - end.roundToPx() - val height = placeable.height - top.roundToPx() - bottom.roundToPx() + val width = placeable.width + start.roundToPx() + end.roundToPx() + val height = placeable.height + top.roundToPx() + bottom.roundToPx() layout(width, height) { - placeable.place(-start.roundToPx(), -top.roundToPx()) + placeable.place(start.roundToPx(), top.roundToPx()) } } } From df137d93b5af558d9741eddb80503ec9233319d0 Mon Sep 17 00:00:00 2001 From: domonkosadam Date: Tue, 25 Nov 2025 17:48:19 +0100 Subject: [PATCH 20/33] Implement correct shadow handling --- .../features/notebook/NotebookScreen.kt | 4 +- .../horizonui/foundation/HorizonBorder.kt | 101 +++++++++++------- 2 files changed, 66 insertions(+), 39 deletions(-) diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt index 117b22c76c..0beaa87994 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt @@ -64,7 +64,7 @@ import com.instructure.horizon.horizonui.foundation.HorizonCornerRadius import com.instructure.horizon.horizonui.foundation.HorizonSpace import com.instructure.horizon.horizonui.foundation.HorizonTypography import com.instructure.horizon.horizonui.foundation.SpaceSize -import com.instructure.horizon.horizonui.foundation.horizonBorderShadow +import com.instructure.horizon.horizonui.foundation.horizonBorder import com.instructure.horizon.horizonui.molecules.Button import com.instructure.horizon.horizonui.molecules.ButtonColor import com.instructure.horizon.horizonui.molecules.ButtonHeight @@ -290,7 +290,7 @@ private fun NoteContent( Column( modifier = Modifier .fillMaxWidth() - .horizonBorderShadow(colorResource(note.type.color), 12.dp, 0.dp, 0.dp, 12.dp, 16.dp) + .horizonBorder(colorResource(note.type.color), 12.dp, 1.dp, 1.dp, 12.dp, 16.dp) .background( color = HorizonColors.PrimitivesWhite.white10(), shape = HorizonCornerRadius.level2, diff --git a/libs/horizon/src/main/java/com/instructure/horizon/horizonui/foundation/HorizonBorder.kt b/libs/horizon/src/main/java/com/instructure/horizon/horizonui/foundation/HorizonBorder.kt index 08eb53ee3b..cdcd3e5b5c 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/horizonui/foundation/HorizonBorder.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/horizonui/foundation/HorizonBorder.kt @@ -5,11 +5,8 @@ import androidx.compose.foundation.layout.padding import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.runtime.Composable import androidx.compose.ui.Modifier -import androidx.compose.ui.draw.drawBehind import androidx.compose.ui.draw.dropShadow -import androidx.compose.ui.geometry.CornerRadius import androidx.compose.ui.geometry.Offset -import androidx.compose.ui.geometry.Size import androidx.compose.ui.graphics.Color import androidx.compose.ui.layout.layout import androidx.compose.ui.unit.Dp @@ -23,36 +20,46 @@ object HorizonBorder { @Composable fun Modifier.horizonBorder( color: Color, - start: Dp, - top: Dp, - end: Dp, - bottom: Dp, - cornerRadius: Dp, + start: Dp = 0.dp, + top: Dp = 0.dp, + end: Dp = 0.dp, + bottom: Dp = 0.dp, + cornerRadius: Dp = 0.dp, ): Modifier { - return drawBehind { - drawRoundRect( - color = color, - topLeft = Offset( - x = -start.toPx(), - y = -top.toPx() - ), - size = Size( - width = size.width + start.toPx() + end.toPx(), - height = size.height + top.toPx() + bottom.toPx() - ), - cornerRadius = CornerRadius(cornerRadius.toPx(), cornerRadius.toPx()), - ) - } + return this + .padding(start, top, end, bottom) + .layout { measurable, constraints -> + val placeable = measurable.measure(constraints) + val width = placeable.width - start.roundToPx() - end.roundToPx() + val height = placeable.height - top.roundToPx() - bottom.roundToPx() + + layout(width, height) { + placeable.place(-start.roundToPx(), -top.roundToPx()) + } + } + .dropShadow(RoundedCornerShape(cornerRadius)) { + this.color = color.copy(0.1f) + this.radius = 0f + } + .layout { measurable, constraints -> + val placeable = measurable.measure(constraints) + val width = placeable.width + start.roundToPx() + end.roundToPx() + val height = placeable.height + top.roundToPx() + bottom.roundToPx() + + layout(width, height) { + placeable.place(start.roundToPx(), top.roundToPx()) + } + } } @Composable fun Modifier.horizonBorderShadow( color: Color, - start: Dp, - top: Dp, - end: Dp, - bottom: Dp, - cornerRadius: Dp, + start: Dp = 0.dp, + top: Dp = 0.dp, + end: Dp = 0.dp, + bottom: Dp = 0.dp, + cornerRadius: Dp = 0.dp, ): Modifier { val maxShadow = maxOf(start, top, end, bottom) @@ -70,6 +77,26 @@ fun Modifier.horizonBorderShadow( .dropShadow(RoundedCornerShape(cornerRadius)) { this.color = color.copy(0.1f) this.radius = maxShadow.toPx() / 2 + + val offsetX = if (start == 0.dp) { + this.radius = maxShadow.toPx() / 4 + radius + } else if (end == 0.dp) { + this.radius = maxShadow.toPx() / 4 + -radius + } else { + 0f + } + val offsetY = if (top == 0.dp) { + this.radius = maxShadow.toPx() / 4 + radius + } else if (bottom == 0.dp) { + this.radius = maxShadow.toPx() / 4 + -radius + } else { + 0f + } + this.offset = Offset(offsetX, offsetY) } .layout { measurable, constraints -> val placeable = measurable.measure(constraints) @@ -85,9 +112,9 @@ fun Modifier.horizonBorderShadow( @Composable fun Modifier.horizonBorder( color: Color, - horizontal: Dp, - vertical: Dp, - cornerRadius: Dp, + horizontal: Dp = 0.dp, + vertical: Dp = 0.dp, + cornerRadius: Dp = 0.dp, ): Modifier { return horizonBorder(color, horizontal, vertical, horizontal, vertical, cornerRadius) } @@ -95,8 +122,8 @@ fun Modifier.horizonBorder( @Composable fun Modifier.horizonBorder( color: Color, - all: Dp, - cornerRadius: Dp, + all: Dp = 0.dp, + cornerRadius: Dp = 0.dp, ): Modifier { return horizonBorder(color, all, all, cornerRadius) } @@ -104,9 +131,9 @@ fun Modifier.horizonBorder( @Composable fun Modifier.horizonBorderShadow( color: Color, - horizontal: Dp, - vertical: Dp, - cornerRadius: Dp, + horizontal: Dp = 0.dp, + vertical: Dp = 0.dp, + cornerRadius: Dp = 0.dp, ): Modifier { return horizonBorderShadow(color, horizontal, vertical, horizontal, vertical, cornerRadius) } @@ -114,8 +141,8 @@ fun Modifier.horizonBorderShadow( @Composable fun Modifier.horizonBorderShadow( color: Color, - all: Dp, - cornerRadius: Dp, + all: Dp = 0.dp, + cornerRadius: Dp = 0.dp, ): Modifier { return horizonBorderShadow(color, all, all, cornerRadius) } From 99f7d6a35a092d801a8ea399359f40fa7fc3d0d7 Mon Sep 17 00:00:00 2001 From: domonkosadam Date: Tue, 25 Nov 2025 17:59:14 +0100 Subject: [PATCH 21/33] Fix shadow attributes --- .../horizon/horizonui/foundation/HorizonBorder.kt | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/libs/horizon/src/main/java/com/instructure/horizon/horizonui/foundation/HorizonBorder.kt b/libs/horizon/src/main/java/com/instructure/horizon/horizonui/foundation/HorizonBorder.kt index cdcd3e5b5c..a491070ee9 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/horizonui/foundation/HorizonBorder.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/horizonui/foundation/HorizonBorder.kt @@ -38,7 +38,7 @@ fun Modifier.horizonBorder( } } .dropShadow(RoundedCornerShape(cornerRadius)) { - this.color = color.copy(0.1f) + this.color = color this.radius = 0f } .layout { measurable, constraints -> @@ -76,22 +76,22 @@ fun Modifier.horizonBorderShadow( } .dropShadow(RoundedCornerShape(cornerRadius)) { this.color = color.copy(0.1f) - this.radius = maxShadow.toPx() / 2 + this.radius = maxShadow.toPx() val offsetX = if (start == 0.dp) { - this.radius = maxShadow.toPx() / 4 + this.radius = maxShadow.toPx() / 2 radius } else if (end == 0.dp) { - this.radius = maxShadow.toPx() / 4 + this.radius = maxShadow.toPx() / 2 -radius } else { 0f } val offsetY = if (top == 0.dp) { - this.radius = maxShadow.toPx() / 4 + this.radius = maxShadow.toPx() / 2 radius } else if (bottom == 0.dp) { - this.radius = maxShadow.toPx() / 4 + this.radius = maxShadow.toPx() / 2 -radius } else { 0f From 3cef77ccaf94661a2c31ed702af68953cf5cc7c9 Mon Sep 17 00:00:00 2001 From: domonkosadam Date: Tue, 25 Nov 2025 18:00:46 +0100 Subject: [PATCH 22/33] Fix colors --- .../com/instructure/horizon/features/notebook/NotebookScreen.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt index 0beaa87994..36ad93ea28 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt @@ -290,7 +290,7 @@ private fun NoteContent( Column( modifier = Modifier .fillMaxWidth() - .horizonBorder(colorResource(note.type.color), 12.dp, 1.dp, 1.dp, 12.dp, 16.dp) + .horizonBorder(colorResource(note.type.color).copy(alpha = 0.1f), 12.dp, 1.dp, 1.dp, 12.dp, 16.dp) .background( color = HorizonColors.PrimitivesWhite.white10(), shape = HorizonCornerRadius.level2, From 1b309f2b8bdb671075bc5934c94e806e2fd75594 Mon Sep 17 00:00:00 2001 From: domonkosadam Date: Wed, 26 Nov 2025 10:15:34 +0100 Subject: [PATCH 23/33] Border improvements --- .../features/notebook/NotebookScreen.kt | 63 +++++++++++-------- .../horizonui/foundation/HorizonBorder.kt | 7 --- 2 files changed, 36 insertions(+), 34 deletions(-) diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt index 36ad93ea28..8281e085c5 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt @@ -65,17 +65,18 @@ import com.instructure.horizon.horizonui.foundation.HorizonSpace import com.instructure.horizon.horizonui.foundation.HorizonTypography import com.instructure.horizon.horizonui.foundation.SpaceSize import com.instructure.horizon.horizonui.foundation.horizonBorder +import com.instructure.horizon.horizonui.foundation.horizonBorderShadow import com.instructure.horizon.horizonui.molecules.Button import com.instructure.horizon.horizonui.molecules.ButtonColor import com.instructure.horizon.horizonui.molecules.ButtonHeight import com.instructure.horizon.horizonui.molecules.ButtonWidth import com.instructure.horizon.horizonui.molecules.DropdownChip import com.instructure.horizon.horizonui.molecules.DropdownItem -import com.instructure.horizon.horizonui.molecules.HorizonDivider import com.instructure.horizon.horizonui.molecules.Spinner import com.instructure.horizon.horizonui.organisms.CollapsableHeaderScreen import com.instructure.horizon.horizonui.platform.LoadingStateWrapper import com.instructure.horizon.navigation.MainNavigationRoute +import com.instructure.pandautils.compose.modifiers.conditional import com.instructure.pandautils.utils.ViewStyler import com.instructure.pandautils.utils.getActivityOrNull import com.instructure.pandautils.utils.localisedFormat @@ -118,8 +119,15 @@ fun NotebookScreen( state, scrollState, Modifier + .background(HorizonColors.Surface.pagePrimary()) .clip(HorizonCornerRadius.level5) - .background(HorizonColors.Surface.pageSecondary()), + .conditional(scrollState.canScrollBackward) { + horizonBorderShadow( + HorizonColors.Surface.inversePrimary(), + bottom = 1.dp, + ) + } + .background(HorizonColors.Surface.pageSecondary()) ) } @@ -239,32 +247,26 @@ private fun FilterContent( if (state.selectedCourse == null) allCoursesItem else courseItems.find { it.value == state.selectedCourse } - Column { - Row( - modifier = modifier - .fillMaxWidth() - .padding(16.dp), - horizontalArrangement = Arrangement.spacedBy(8.dp) - ) { - if (state.showCourseFilter) { - DropdownChip( - items = courseItems, - selectedItem = selectedCourseItem, - onItemSelected = { item -> state.onCourseSelected(item?.value) }, - placeholder = stringResource(R.string.notebookFilterCoursePlaceholder), - dropdownWidth = 178.dp, - verticalPadding = 6.dp, - modifier = Modifier.weight(1f, false) - ) - } - - if (state.showNoteTypeFilter) { - NotebookTypeSelect(state.selectedFilter, state.onFilterSelected, false, true) - } + Row( + modifier = modifier + .fillMaxWidth() + .padding(16.dp), + horizontalArrangement = Arrangement.spacedBy(8.dp) + ) { + if (state.showCourseFilter) { + DropdownChip( + items = courseItems, + selectedItem = selectedCourseItem, + onItemSelected = { item -> state.onCourseSelected(item?.value) }, + placeholder = stringResource(R.string.notebookFilterCoursePlaceholder), + dropdownWidth = 178.dp, + verticalPadding = 6.dp, + modifier = Modifier.weight(1f, false) + ) } - if (scrollState.canScrollBackward) { - HorizonDivider() + if (state.showNoteTypeFilter) { + NotebookTypeSelect(state.selectedFilter, state.onFilterSelected, false, true) } } } @@ -290,7 +292,14 @@ private fun NoteContent( Column( modifier = Modifier .fillMaxWidth() - .horizonBorder(colorResource(note.type.color).copy(alpha = 0.1f), 12.dp, 1.dp, 1.dp, 12.dp, 16.dp) + .horizonBorder( + colorResource(note.type.color).copy(alpha = 0.1f), + 12.dp, + 1.dp, + 1.dp, + 12.dp, + 16.dp + ) .background( color = HorizonColors.PrimitivesWhite.white10(), shape = HorizonCornerRadius.level2, diff --git a/libs/horizon/src/main/java/com/instructure/horizon/horizonui/foundation/HorizonBorder.kt b/libs/horizon/src/main/java/com/instructure/horizon/horizonui/foundation/HorizonBorder.kt index a491070ee9..91816e8464 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/horizonui/foundation/HorizonBorder.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/horizonui/foundation/HorizonBorder.kt @@ -3,7 +3,6 @@ package com.instructure.horizon.horizonui.foundation import androidx.compose.foundation.BorderStroke import androidx.compose.foundation.layout.padding import androidx.compose.foundation.shape.RoundedCornerShape -import androidx.compose.runtime.Composable import androidx.compose.ui.Modifier import androidx.compose.ui.draw.dropShadow import androidx.compose.ui.geometry.Offset @@ -17,7 +16,6 @@ object HorizonBorder { fun level2(color: Color = HorizonColors.LineAndBorder.lineStroke()) = BorderStroke(2.dp, color) } -@Composable fun Modifier.horizonBorder( color: Color, start: Dp = 0.dp, @@ -52,7 +50,6 @@ fun Modifier.horizonBorder( } } -@Composable fun Modifier.horizonBorderShadow( color: Color, start: Dp = 0.dp, @@ -109,7 +106,6 @@ fun Modifier.horizonBorderShadow( } } -@Composable fun Modifier.horizonBorder( color: Color, horizontal: Dp = 0.dp, @@ -119,7 +115,6 @@ fun Modifier.horizonBorder( return horizonBorder(color, horizontal, vertical, horizontal, vertical, cornerRadius) } -@Composable fun Modifier.horizonBorder( color: Color, all: Dp = 0.dp, @@ -128,7 +123,6 @@ fun Modifier.horizonBorder( return horizonBorder(color, all, all, cornerRadius) } -@Composable fun Modifier.horizonBorderShadow( color: Color, horizontal: Dp = 0.dp, @@ -138,7 +132,6 @@ fun Modifier.horizonBorderShadow( return horizonBorderShadow(color, horizontal, vertical, horizontal, vertical, cornerRadius) } -@Composable fun Modifier.horizonBorderShadow( color: Color, all: Dp = 0.dp, From e280c2bd9096b65a3b4486f8a130d44c8d8623a4 Mon Sep 17 00:00:00 2001 From: domonkosadam Date: Wed, 26 Nov 2025 10:36:43 +0100 Subject: [PATCH 24/33] Implement ui layout --- .../features/notebook/NotebookScreen.kt | 34 +++++++--- .../common/composable/NotebookPill.kt | 65 ------------------- .../common/composable/NotebookTypeSelect.kt | 6 +- .../horizonui/molecules/DropdownChip.kt | 29 +++++---- 4 files changed, 47 insertions(+), 87 deletions(-) delete mode 100644 libs/horizon/src/main/java/com/instructure/horizon/features/notebook/common/composable/NotebookPill.kt diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt index 8281e085c5..0d5df7b5e3 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt @@ -54,7 +54,6 @@ import com.instructure.canvasapi2.utils.ContextKeeper import com.instructure.horizon.R import com.instructure.horizon.features.notebook.common.composable.NotebookAppBar import com.instructure.horizon.features.notebook.common.composable.NotebookHighlightedText -import com.instructure.horizon.features.notebook.common.composable.NotebookPill import com.instructure.horizon.features.notebook.common.composable.NotebookTypeSelect import com.instructure.horizon.features.notebook.common.composable.toNotebookLocalisedDateFormat import com.instructure.horizon.features.notebook.common.model.Note @@ -155,7 +154,10 @@ fun NotebookScreen( } else { items(state.notes) { note -> Column { - NoteContent(note) { + val courseName = if (state.showCourseFilter) { + state.courses.firstOrNull { it.courseId == note.courseId }?.courseName + } else null + NoteContent(note, courseName) { if (state.navigateToEdit) { mainNavController.navigate( NotebookRoute.EditNotebook( @@ -287,6 +289,7 @@ private fun LoadingContent() { @Composable private fun NoteContent( note: Note, + courseName: String?, onClick: () -> Unit, ) { Column( @@ -311,10 +314,13 @@ private fun NoteContent( .fillMaxWidth() .padding(24.dp) ) { - Text( - text = note.updatedAt.localisedFormat("MMM d, yyyy"), - style = HorizonTypography.labelSmall, - color = HorizonColors.Text.timestamp() + NotebookTypeSelect( + note.type, + verticalPadding = 2.dp, + onSelect = {}, + showIcons = true, + enabled = false, + showAllOption = false ) HorizonSpace(SpaceSize.SPACE_16) @@ -336,10 +342,22 @@ private fun NoteContent( overflow = TextOverflow.Ellipsis, ) - HorizonSpace(SpaceSize.SPACE_16) + HorizonSpace(SpaceSize.SPACE_8) } - NotebookPill(note.type) + Text( + text = note.updatedAt.localisedFormat("MMM d, yyyy"), + style = HorizonTypography.labelMediumBold, + color = HorizonColors.Text.timestamp() + ) + + if (courseName != null) { + Text( + text = courseName, + style = HorizonTypography.labelMediumBold, + color = HorizonColors.Text.timestamp() + ) + } } } } diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/common/composable/NotebookPill.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/common/composable/NotebookPill.kt deleted file mode 100644 index da6c3daa2b..0000000000 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/common/composable/NotebookPill.kt +++ /dev/null @@ -1,65 +0,0 @@ -/* - * Copyright (C) 2025 - present Instructure, Inc. - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, version 3 of the License. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see . - * - */ -package com.instructure.horizon.features.notebook.common.composable - -import androidx.compose.runtime.Composable -import androidx.compose.ui.Modifier -import androidx.compose.ui.platform.LocalContext -import androidx.compose.ui.res.stringResource -import androidx.compose.ui.tooling.preview.Preview -import com.instructure.canvasapi2.utils.ContextKeeper -import com.instructure.horizon.features.notebook.common.model.NotebookType -import com.instructure.horizon.horizonui.molecules.Pill -import com.instructure.horizon.horizonui.molecules.PillCase -import com.instructure.horizon.horizonui.molecules.PillSize -import com.instructure.horizon.horizonui.molecules.PillStyle -import com.instructure.horizon.horizonui.molecules.PillType - -@Composable -fun NotebookPill( - type: NotebookType, - modifier: Modifier = Modifier -) { - val pillType = when (type) { - NotebookType.Confusing -> PillType.DANGER - NotebookType.Important -> PillType.INSTITUTION - } - - Pill( - label = stringResource(type.labelRes), - style = PillStyle.OUTLINE, - type = pillType, - case = PillCase.UPPERCASE, - size = PillSize.REGULAR, - iconRes = type.iconRes, - modifier = modifier - ) -} - -@Composable -@Preview -private fun NotebookPillConfusingPreview() { - ContextKeeper.appContext = LocalContext.current - NotebookPill(type = NotebookType.Confusing) -} - -@Composable -@Preview -private fun NotebookPillImportantPreview() { - ContextKeeper.appContext = LocalContext.current - NotebookPill(type = NotebookType.Important) -} \ No newline at end of file diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/common/composable/NotebookTypeSelect.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/common/composable/NotebookTypeSelect.kt index 5fd78f56dc..12165e50b3 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/common/composable/NotebookTypeSelect.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/common/composable/NotebookTypeSelect.kt @@ -23,6 +23,7 @@ import androidx.compose.ui.graphics.Color import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.res.stringResource import androidx.compose.ui.tooling.preview.Preview +import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp import com.instructure.horizon.R import com.instructure.horizon.features.notebook.common.model.NotebookType @@ -37,6 +38,8 @@ fun NotebookTypeSelect( showIcons: Boolean, showAllOption: Boolean, modifier: Modifier = Modifier, + enabled: Boolean = true, + verticalPadding: Dp = 6.dp ) { val context = LocalContext.current val defaultBackgroundColor = HorizonColors.PrimitivesGrey.grey12() @@ -84,8 +87,9 @@ fun NotebookTypeSelect( onItemSelected = { item -> onSelect(item?.value) }, placeholder = stringResource(R.string.notebookFilterTypePlaceholder), dropdownWidth = 178.dp, - verticalPadding = 6.dp, + verticalPadding = verticalPadding, showIconCollapsed = showIcons, + enabled = enabled, borderColor = if (showIcons) { selectedTypeItem?.iconTint ?: HorizonColors.LineAndBorder.lineStroke() } else { diff --git a/libs/horizon/src/main/java/com/instructure/horizon/horizonui/molecules/DropdownChip.kt b/libs/horizon/src/main/java/com/instructure/horizon/horizonui/molecules/DropdownChip.kt index feae7094cf..964e6a273b 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/horizonui/molecules/DropdownChip.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/horizonui/molecules/DropdownChip.kt @@ -76,6 +76,7 @@ fun DropdownChip( dropdownWidth: Dp? = null, placeholder: String, showIconCollapsed: Boolean = false, + enabled: Boolean = true, borderColor: Color = HorizonColors.LineAndBorder.lineStroke(), contentColor: Color = HorizonColors.Text.body(), verticalPadding: Dp = 0.dp @@ -108,7 +109,7 @@ fun DropdownChip( HorizonCornerRadius.level1 ) .clip(HorizonCornerRadius.level1) - .clickable { isMenuOpen = !isMenuOpen } + .conditional(enabled) { clickable { isMenuOpen = !isMenuOpen } } .padding(horizontal = 8.dp, vertical = 2.dp) .onGloballyPositioned { heightInPx = it.size.height @@ -141,20 +142,22 @@ fun DropdownChip( modifier = Modifier .weight(1f, false) .padding( - end = 2.dp, - top = verticalPadding, - bottom = verticalPadding - ) + end = 2.dp, + top = verticalPadding, + bottom = verticalPadding + ) ) - Icon( - painter = painterResource(R.drawable.keyboard_arrow_down), - contentDescription = null, - modifier = Modifier - .size(16.dp) - .rotate(iconRotation.value.toFloat()), - tint = contentColor - ) + if (enabled) { + Icon( + painter = painterResource(R.drawable.keyboard_arrow_down), + contentDescription = null, + modifier = Modifier + .size(16.dp) + .rotate(iconRotation.value.toFloat()), + tint = contentColor + ) + } } InputDropDownPopup( From 59715e1d0efe4706e65b916b9269e97a00aefd29 Mon Sep 17 00:00:00 2001 From: domonkosadam Date: Wed, 26 Nov 2025 12:55:32 +0100 Subject: [PATCH 25/33] Implement delete --- .../features/notebook/NotebookRepository.kt | 4 ++ .../features/notebook/NotebookScreen.kt | 55 +++++++++++++++---- .../features/notebook/NotebookUiState.kt | 4 ++ .../features/notebook/NotebookViewModel.kt | 18 ++++++ .../notebook/addedit/AddEditNoteScreen.kt | 35 ++++-------- ...dEditNoteScreenDeleteConfirmationDialog.kt | 32 +++++++++++ .../horizon/horizonui/molecules/IconButton.kt | 51 +++++++++++++++++ 7 files changed, 163 insertions(+), 36 deletions(-) create mode 100644 libs/horizon/src/main/java/com/instructure/horizon/features/notebook/common/composable/AddEditNoteScreenDeleteConfirmationDialog.kt diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookRepository.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookRepository.kt index 88cb454dac..d3240e99ce 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookRepository.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookRepository.kt @@ -94,4 +94,8 @@ class NotebookRepository @Inject constructor( forceNetwork = forceNetwork ).dataOrNull.orEmpty() } + + suspend fun deleteNote(noteId: String) { + redwoodApiManager.deleteNote(noteId) + } } \ No newline at end of file diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt index 0d5df7b5e3..fff24934c2 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt @@ -52,6 +52,7 @@ import androidx.navigation.NavHostController import com.instructure.canvasapi2.managers.graphql.horizon.CourseWithProgress import com.instructure.canvasapi2.utils.ContextKeeper import com.instructure.horizon.R +import com.instructure.horizon.features.notebook.common.composable.NoteDeleteConfirmationDialog import com.instructure.horizon.features.notebook.common.composable.NotebookAppBar import com.instructure.horizon.features.notebook.common.composable.NotebookHighlightedText import com.instructure.horizon.features.notebook.common.composable.NotebookTypeSelect @@ -71,6 +72,9 @@ import com.instructure.horizon.horizonui.molecules.ButtonHeight import com.instructure.horizon.horizonui.molecules.ButtonWidth import com.instructure.horizon.horizonui.molecules.DropdownChip import com.instructure.horizon.horizonui.molecules.DropdownItem +import com.instructure.horizon.horizonui.molecules.IconButtonColor +import com.instructure.horizon.horizonui.molecules.IconButtonSize +import com.instructure.horizon.horizonui.molecules.LoadingIconButton import com.instructure.horizon.horizonui.molecules.Spinner import com.instructure.horizon.horizonui.organisms.CollapsableHeaderScreen import com.instructure.horizon.horizonui.platform.LoadingStateWrapper @@ -96,6 +100,14 @@ fun NotebookScreen( val scrollState = rememberLazyListState() + NoteDeleteConfirmationDialog( + showDialog = state.showDeleteConfirmationForNote != null, + dismissDialog = { state.updateShowDeleteConfirmation(null) }, + onDeleteSelected = { + state.deleteNote(state.showDeleteConfirmationForNote) + } + ) + CollapsableHeaderScreen( modifier = Modifier.background(HorizonColors.Surface.pagePrimary()), headerContent = { @@ -157,7 +169,9 @@ fun NotebookScreen( val courseName = if (state.showCourseFilter) { state.courses.firstOrNull { it.courseId == note.courseId }?.courseName } else null - NoteContent(note, courseName) { + NoteContent(note, courseName, state.deleteLoadingNote, onDeleteClick = { + state.updateShowDeleteConfirmation(note) + }) { if (state.navigateToEdit) { mainNavController.navigate( NotebookRoute.EditNotebook( @@ -290,6 +304,8 @@ private fun LoadingContent() { private fun NoteContent( note: Note, courseName: String?, + deleteLoading: Note?, + onDeleteClick: () -> Unit, onClick: () -> Unit, ) { Column( @@ -307,6 +323,7 @@ private fun NoteContent( color = HorizonColors.PrimitivesWhite.white10(), shape = HorizonCornerRadius.level2, ) + .clip(HorizonCornerRadius.level2) .clickable { onClick() } ) { Column( @@ -345,17 +362,33 @@ private fun NoteContent( HorizonSpace(SpaceSize.SPACE_8) } - Text( - text = note.updatedAt.localisedFormat("MMM d, yyyy"), - style = HorizonTypography.labelMediumBold, - color = HorizonColors.Text.timestamp() - ) + Row( + verticalAlignment = Alignment.Bottom + ) { + Column( + modifier = Modifier.weight(1f) + ){ + Text( + text = note.updatedAt.localisedFormat("MMM d, yyyy"), + style = HorizonTypography.labelMediumBold, + color = HorizonColors.Text.timestamp() + ) - if (courseName != null) { - Text( - text = courseName, - style = HorizonTypography.labelMediumBold, - color = HorizonColors.Text.timestamp() + if (courseName != null) { + Text( + text = courseName, + style = HorizonTypography.labelMediumBold, + color = HorizonColors.Text.timestamp() + ) + } + } + + LoadingIconButton( + iconRes = R.drawable.delete, + color = IconButtonColor.InverseDanger, + size = IconButtonSize.SMALL, + onClick = { onDeleteClick() }, + loading = note == deleteLoading ) } } diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookUiState.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookUiState.kt index af097acf76..901b289d91 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookUiState.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookUiState.kt @@ -37,4 +37,8 @@ data class NotebookUiState( val navigateToEdit: Boolean = false, val showNoteTypeFilter: Boolean = true, val showCourseFilter: Boolean = true, + val showDeleteConfirmationForNote: Note? = null, + val updateShowDeleteConfirmation: (Note?) -> Unit = {}, + val deleteNote: (Note?) -> Unit = {}, + val deleteLoadingNote: Note? = null ) \ No newline at end of file diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookViewModel.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookViewModel.kt index e52ca48adb..f36d491956 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookViewModel.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookViewModel.kt @@ -71,6 +71,8 @@ class NotebookViewModel @Inject constructor( showFilters = showFilters, showCourseFilter = courseId == null, navigateToEdit = navigateToEdit, + updateShowDeleteConfirmation = ::updateShowDeleteConfirmation, + deleteNote = ::deleteNote ) ) val uiState = _uiState.asStateFlow() @@ -196,4 +198,20 @@ class NotebookViewModel @Inject constructor( val objectId = savedStateHandle.get(NotebookRoute.Notebook.OBJECT_ID) ?: return null return Pair(objectType, objectId) } + + private fun updateShowDeleteConfirmation(note: Note?) { + _uiState.update { it.copy(showDeleteConfirmationForNote = note) } + } + + private fun deleteNote(note: Note?) { + if (note != null) { + viewModelScope.tryLaunch { + _uiState.update { it.copy(deleteLoadingNote = note) } + repository.deleteNote(note.id) + _uiState.update { it.copy(deleteLoadingNote = null, notes = it.notes.filterNot { it == note }) } + } catch { + _uiState.update { it.copy(deleteLoadingNote = null) } + } + } + } } \ No newline at end of file diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/addedit/AddEditNoteScreen.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/addedit/AddEditNoteScreen.kt index 9675d870f5..e2563e208e 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/addedit/AddEditNoteScreen.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/addedit/AddEditNoteScreen.kt @@ -48,6 +48,7 @@ import com.instructure.canvasapi2.managers.graphql.horizon.redwood.NoteHighlight import com.instructure.canvasapi2.managers.graphql.horizon.redwood.NoteHighlightedDataTextPosition import com.instructure.canvasapi2.utils.ContextKeeper import com.instructure.horizon.R +import com.instructure.horizon.features.notebook.common.composable.NoteDeleteConfirmationDialog import com.instructure.horizon.features.notebook.common.composable.NotebookHighlightedText import com.instructure.horizon.features.notebook.common.composable.NotebookTypeSelect import com.instructure.horizon.features.notebook.common.model.NotebookType @@ -99,7 +100,15 @@ fun AddEditNoteScreen( if (state.isLoading) { AddEditNoteLoading(padding) } else { - AddEditNoteScreenDeleteConfirmationDialog(state, navController) + NoteDeleteConfirmationDialog( + showDialog = state.showDeleteConfirmationDialog, + onDeleteSelected = { + state.onDeleteNote?.invoke { + navController.popBackStack() + } + }, + dismissDialog = { state.updateDeleteConfirmationDialog(false) } + ) AddEditNoteScreenExitConfirmationDialog(state, navController) AddEditNoteContent(state, padding) } @@ -233,30 +242,6 @@ private fun AddEditNoteLoading(padding: PaddingValues) { } } -@Composable -private fun AddEditNoteScreenDeleteConfirmationDialog( - state: AddEditNoteUiState, - navController: NavHostController -) { - if (state.showDeleteConfirmationDialog) { - Modal( - ModalDialogState( - title = stringResource(R.string.deleteNoteConfirmationTitle), - message = stringResource(R.string.deleteNoteConfirmationMessage), - primaryButtonTitle = stringResource(R.string.deleteNoteConfirmationDeleteLabel), - primaryButtonClick = { - state.updateDeleteConfirmationDialog(false) - state.onDeleteNote?.invoke { navController.popBackStack() } - }, - secondaryButtonTitle = stringResource(R.string.deleteNoteConfirmationCancelLabel), - secondaryButtonClick = { state.updateDeleteConfirmationDialog(false) } - - ), - onDismiss = { state.updateDeleteConfirmationDialog(false) } - ) - } -} - @Composable private fun AddEditNoteScreenExitConfirmationDialog( state: AddEditNoteUiState, diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/common/composable/AddEditNoteScreenDeleteConfirmationDialog.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/common/composable/AddEditNoteScreenDeleteConfirmationDialog.kt new file mode 100644 index 0000000000..97bbccc2a0 --- /dev/null +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/common/composable/AddEditNoteScreenDeleteConfirmationDialog.kt @@ -0,0 +1,32 @@ +package com.instructure.horizon.features.notebook.common.composable + +import androidx.compose.runtime.Composable +import androidx.compose.ui.res.stringResource +import com.instructure.horizon.R +import com.instructure.horizon.horizonui.organisms.Modal +import com.instructure.horizon.horizonui.organisms.ModalDialogState + +@Composable +fun NoteDeleteConfirmationDialog( + showDialog: Boolean, + onDeleteSelected: () -> Unit, + dismissDialog: () -> Unit, +) { + if (showDialog) { + Modal( + ModalDialogState( + title = stringResource(R.string.deleteNoteConfirmationTitle), + message = stringResource(R.string.deleteNoteConfirmationMessage), + primaryButtonTitle = stringResource(R.string.deleteNoteConfirmationDeleteLabel), + primaryButtonClick = { + dismissDialog() + onDeleteSelected() + }, + secondaryButtonTitle = stringResource(R.string.deleteNoteConfirmationCancelLabel), + secondaryButtonClick = { dismissDialog() } + + ), + onDismiss = { dismissDialog() } + ) + } +} \ No newline at end of file diff --git a/libs/horizon/src/main/java/com/instructure/horizon/horizonui/molecules/IconButton.kt b/libs/horizon/src/main/java/com/instructure/horizon/horizonui/molecules/IconButton.kt index a8cc113d15..b523d3aa49 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/horizonui/molecules/IconButton.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/horizonui/molecules/IconButton.kt @@ -16,10 +16,12 @@ package com.instructure.horizon.horizonui.molecules import androidx.annotation.DrawableRes +import androidx.compose.animation.animateContentSize import androidx.compose.foundation.background import androidx.compose.foundation.border import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.offset +import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size import androidx.compose.foundation.shape.CircleShape import androidx.compose.material3.Icon @@ -170,6 +172,55 @@ fun IconButton( } } +@Composable +fun LoadingIconButton( + @DrawableRes iconRes: Int, + loading: Boolean, + modifier: Modifier = Modifier, + size: IconButtonSize = IconButtonSize.NORMAL, + color: IconButtonColor = IconButtonColor.Black, + elevation: Dp? = null, + enabled: Boolean = true, + contentDescription: String? = null, + onClick: () -> Unit = {}, + contentAlignment: Alignment = Alignment.Center, + badge: @Composable (() -> Unit)? = null +) { + Box( + contentAlignment = contentAlignment, + modifier = modifier + .animateContentSize() + ) { + if (loading) { + Box( + contentAlignment = Alignment.Center, + modifier = Modifier + .background(color = color.backgroundColor, shape = HorizonCornerRadius.level6) + ) { + Spinner( + size = SpinnerSize.EXTRA_SMALL, + color = color.iconColor, + modifier = Modifier + .align(Alignment.Center) + .padding(8.dp), + ) + } + } else { + IconButton( + iconRes = iconRes, + modifier = modifier, + size = size, + color = color, + elevation = elevation, + enabled = enabled, + contentDescription = contentDescription, + onClick = onClick, + badge = badge + ) + } + } +} + @Composable @Preview(showBackground = true) private fun IconButtonPreview() { From 8e8d23dc65805dbfeb2b78d6123c3af106781c30 Mon Sep 17 00:00:00 2001 From: domonkosadam Date: Wed, 26 Nov 2025 13:08:02 +0100 Subject: [PATCH 26/33] Fix UI --- .../horizon/features/notebook/NotebookScreen.kt | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt index fff24934c2..ea46e0fc4a 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt @@ -216,7 +216,6 @@ fun NotebookScreen( width = ButtonWidth.FILL, color = ButtonColor.WhiteWithOutline, onClick = { state.loadNextPage() }, - modifier = Modifier.padding(vertical = 24.dp) ) } } @@ -362,11 +361,9 @@ private fun NoteContent( HorizonSpace(SpaceSize.SPACE_8) } - Row( - verticalAlignment = Alignment.Bottom - ) { + Row { Column( - modifier = Modifier.weight(1f) + modifier = Modifier.weight(1f).align(Alignment.CenterVertically) ){ Text( text = note.updatedAt.localisedFormat("MMM d, yyyy"), @@ -388,7 +385,8 @@ private fun NoteContent( color = IconButtonColor.InverseDanger, size = IconButtonSize.SMALL, onClick = { onDeleteClick() }, - loading = note == deleteLoading + loading = note == deleteLoading, + modifier = Modifier.align(Alignment.Bottom) ) } } From f694bfbc098e68d89dd4c81e4bcd6c1058b4973d Mon Sep 17 00:00:00 2001 From: domonkosadam Date: Wed, 26 Nov 2025 14:58:07 +0100 Subject: [PATCH 27/33] implement tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../features/notebook/NotebookScreenUiTest.kt | 339 ++++++++++++++++++ .../features/notebook/NotebookScreen.kt | 28 +- .../notebook/NotebookRepositoryTest.kt | 25 ++ .../notebook/NotebookViewModelTest.kt | 72 ++++ 4 files changed, 458 insertions(+), 6 deletions(-) create mode 100644 libs/horizon/src/androidTest/java/com/instructure/horizon/ui/features/notebook/NotebookScreenUiTest.kt diff --git a/libs/horizon/src/androidTest/java/com/instructure/horizon/ui/features/notebook/NotebookScreenUiTest.kt b/libs/horizon/src/androidTest/java/com/instructure/horizon/ui/features/notebook/NotebookScreenUiTest.kt new file mode 100644 index 0000000000..8f7a97a0e3 --- /dev/null +++ b/libs/horizon/src/androidTest/java/com/instructure/horizon/ui/features/notebook/NotebookScreenUiTest.kt @@ -0,0 +1,339 @@ +/* + * Copyright (C) 2025 - present Instructure, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, version 3 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ +package com.instructure.horizon.ui.features.notebook + +import androidx.compose.ui.test.assertIsDisplayed +import androidx.compose.ui.test.hasText +import androidx.compose.ui.test.junit4.createComposeRule +import androidx.compose.ui.test.onNodeWithText +import androidx.navigation.compose.rememberNavController +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.platform.app.InstrumentationRegistry +import com.instructure.canvasapi2.managers.graphql.horizon.CourseWithProgress +import com.instructure.canvasapi2.managers.graphql.horizon.redwood.NoteHighlightedData +import com.instructure.canvasapi2.managers.graphql.horizon.redwood.NoteHighlightedDataRange +import com.instructure.canvasapi2.managers.graphql.horizon.redwood.NoteHighlightedDataTextPosition +import com.instructure.canvasapi2.managers.graphql.horizon.redwood.NoteObjectType +import com.instructure.canvasapi2.utils.ContextKeeper +import com.instructure.horizon.R +import com.instructure.horizon.features.notebook.NotebookScreen +import com.instructure.horizon.features.notebook.NotebookUiState +import com.instructure.horizon.features.notebook.common.model.Note +import com.instructure.horizon.features.notebook.common.model.NotebookType +import com.instructure.horizon.horizonui.platform.LoadingState +import dagger.hilt.android.testing.HiltAndroidTest +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import java.util.Date + +@HiltAndroidTest +@RunWith(AndroidJUnit4::class) +class NotebookScreenUiTest { + @get:Rule + val composeTestRule = createComposeRule() + + private val context = InstrumentationRegistry.getInstrumentation().targetContext + + @Test + fun testEmptyStateDisplaysWhenNoNotes() { + val state = createEmptyState() + + composeTestRule.setContent { + ContextKeeper.appContext = context + val navController = rememberNavController() + NotebookScreen(navController, state) + } + + composeTestRule.onNodeWithText(context.getString(R.string.notesEmptyContentTitle)) + .assertIsDisplayed() + } + + @Test + fun testNoteCardDisplaysHighlightedText() { + val highlightedText = "This is important highlighted text from the course material" + val state = createStateWithNotes( + notes = listOf(createTestNote(highlightedText = highlightedText)) + ) + + composeTestRule.setContent { + ContextKeeper.appContext = context + val navController = rememberNavController() + NotebookScreen(navController, state) + } + + composeTestRule.onNodeWithText(highlightedText, substring = true) + .assertIsDisplayed() + } + + @Test + fun testNoteCardDisplaysUserComment() { + val userComment = "My personal note about this concept" + val state = createStateWithNotes( + notes = listOf(createTestNote(userText = userComment)) + ) + + composeTestRule.setContent { + ContextKeeper.appContext = context + val navController = rememberNavController() + NotebookScreen(navController, state) + } + + composeTestRule.onNodeWithText(userComment, substring = true) + .assertIsDisplayed() + } + + @Test + fun testNoteCardDisplaysDate() { + val state = createStateWithNotes( + notes = listOf(createTestNote()) + ) + + composeTestRule.setContent { + ContextKeeper.appContext = context + val navController = rememberNavController() + NotebookScreen(navController, state) + } + + composeTestRule.onNode(hasText("Jan", substring = true)) + .assertIsDisplayed() + } + + @Test + fun testNoteCardDisplaysTypeImportant() { + val state = createStateWithNotes( + notes = listOf(createTestNote(type = NotebookType.Important)) + ) + + composeTestRule.setContent { + ContextKeeper.appContext = context + val navController = rememberNavController() + NotebookScreen(navController, state) + } + + composeTestRule.onNodeWithText("Important", useUnmergedTree = true) + .assertIsDisplayed() + } + + @Test + fun testNoteCardDisplaysTypeConfusing() { + val state = createStateWithNotes( + notes = listOf(createTestNote(type = NotebookType.Confusing)) + ) + + composeTestRule.setContent { + ContextKeeper.appContext = context + val navController = rememberNavController() + NotebookScreen(navController, state) + } + + composeTestRule.onNodeWithText("Unclear", useUnmergedTree = true) + .assertIsDisplayed() + } + + @Test + fun testCourseFilterDisplayedWhenEnabled() { + val state = createStateWithNotes( + showCourseFilter = true, + courses = listOf(createTestCourse()) + ) + + composeTestRule.setContent { + ContextKeeper.appContext = context + val navController = rememberNavController() + NotebookScreen(navController, state) + } + + composeTestRule.onNodeWithText(context.getString(R.string.notebookFilterCoursePlaceholder), useUnmergedTree = true) + .assertIsDisplayed() + } + + @Test + fun testNoteTypeFilterDisplayed() { + val state = createStateWithNotes( + showNoteTypeFilter = true, + notes = listOf(createTestNote()) + ) + + composeTestRule.setContent { + ContextKeeper.appContext = context + val navController = rememberNavController() + NotebookScreen(navController, state) + } + + composeTestRule.onNodeWithText("All notes", useUnmergedTree = true) + .assertIsDisplayed() + } + + @Test + fun testCourseNameDisplayedWhenCourseFilterVisible() { + val courseName = "Biology 101" + val state = createStateWithNotes( + showCourseFilter = true, + courses = listOf(createTestCourse(name = courseName, id = 123L)), + notes = listOf(createTestNote(courseId = 123L)) + ) + + composeTestRule.setContent { + ContextKeeper.appContext = context + val navController = rememberNavController() + NotebookScreen(navController, state) + } + + composeTestRule.onNodeWithText(courseName, substring = true) + .assertIsDisplayed() + } + + @Test + fun testCourseNameNotDisplayedWhenCourseFilterHidden() { + val courseName = "Biology 101" + val state = createStateWithNotes( + showCourseFilter = false, + courses = listOf(createTestCourse(name = courseName, id = 123L)), + notes = listOf(createTestNote(courseId = 123L)) + ) + + composeTestRule.setContent { + ContextKeeper.appContext = context + val navController = rememberNavController() + NotebookScreen(navController, state) + } + + composeTestRule.onNode(hasText(courseName)) + .assertDoesNotExist() + } + + @Test + fun testEmptyFilteredStateDisplayedWhenFilterApplied() { + val state = createEmptyState(selectedFilter = NotebookType.Important) + + composeTestRule.setContent { + ContextKeeper.appContext = context + val navController = rememberNavController() + NotebookScreen(navController, state) + } + + composeTestRule.onNodeWithText(context.getString(R.string.notesEmptyFilteredContentTitle)) + .assertIsDisplayed() + } + + @Test + fun testMultipleNotesDisplayed() { + val note1 = createTestNote( + id = "1", + highlightedText = "First important concept" + ) + val note2 = createTestNote( + id = "2", + highlightedText = "Second important concept" + ) + val state = createStateWithNotes(notes = listOf(note1, note2)) + + composeTestRule.setContent { + ContextKeeper.appContext = context + val navController = rememberNavController() + NotebookScreen(navController, state) + } + + composeTestRule.onNodeWithText("First important concept", substring = true) + .assertIsDisplayed() + composeTestRule.onNodeWithText("Second important concept", substring = true) + .assertIsDisplayed() + } + + @Test + fun testShowMoreButtonDisplayedWhenHasNextPage() { + val state = createStateWithNotes( + notes = listOf(createTestNote()), + hasNextPage = true + ) + + composeTestRule.setContent { + ContextKeeper.appContext = context + val navController = rememberNavController() + NotebookScreen(navController, state) + } + + composeTestRule.onNodeWithText(context.getString(R.string.showMore)) + .assertIsDisplayed() + } + + private fun createEmptyState( + selectedFilter: NotebookType? = null + ): NotebookUiState { + return NotebookUiState( + loadingState = LoadingState(isLoading = false), + notes = emptyList(), + selectedFilter = selectedFilter, + showCourseFilter = true, + showNoteTypeFilter = true + ) + } + + private fun createStateWithNotes( + notes: List = emptyList(), + showCourseFilter: Boolean = false, + showNoteTypeFilter: Boolean = false, + courses: List = emptyList(), + hasNextPage: Boolean = false, + selectedFilter: NotebookType? = null + ): NotebookUiState { + return NotebookUiState( + loadingState = LoadingState(isLoading = false), + notes = notes, + courses = courses, + showCourseFilter = showCourseFilter, + showNoteTypeFilter = showNoteTypeFilter, + hasNextPage = hasNextPage, + selectedFilter = selectedFilter + ) + } + + private fun createTestNote( + id: String = "note1", + highlightedText: String = "Test highlighted text from course material", + userText: String = "My personal annotation", + type: NotebookType = NotebookType.Important, + courseId: Long = 123L + ): Note { + return Note( + id = id, + highlightedText = NoteHighlightedData( + selectedText = highlightedText, + range = NoteHighlightedDataRange(0, highlightedText.length, "", ""), + textPosition = NoteHighlightedDataTextPosition(0, highlightedText.length) + ), + type = type, + userText = userText, + updatedAt = Date(1706140800000L), + courseId = courseId, + objectType = NoteObjectType.Assignment, + objectId = "assignment123" + ) + } + + private fun createTestCourse( + name: String = "Test Course", + id: Long = 123L + ): CourseWithProgress { + return CourseWithProgress( + courseId = id, + courseName = name, + progress = 0.0 + ) + } +} diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt index ea46e0fc4a..badb3d7994 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt @@ -90,6 +90,18 @@ fun NotebookScreen( viewModel: NotebookViewModel, ) { val state by viewModel.uiState.collectAsState() + + NotebookScreen( + navController = mainNavController, + state = state + ) +} + +@Composable +fun NotebookScreen( + navController: NavHostController, + state: NotebookUiState +) { val activity = LocalContext.current.getActivityOrNull() LaunchedEffect(Unit) { if (activity != null) ViewStyler.setStatusBarColor( @@ -113,7 +125,7 @@ fun NotebookScreen( headerContent = { if (state.showTopBar) { NotebookAppBar( - navigateBack = { mainNavController.popBackStack() }, + navigateBack = { navController.popBackStack() }, centeredTitle = true ) } @@ -125,7 +137,7 @@ fun NotebookScreen( modifier = Modifier .fillMaxSize() ) { - if ((state.showNoteTypeFilter || state.showCourseFilter) && (state.courses.isNotEmpty() || state.selectedCourse != null || state.selectedFilter != null)) { + if ((state.showNoteTypeFilter || state.showCourseFilter)) { FilterContent( state, scrollState, @@ -173,7 +185,7 @@ fun NotebookScreen( state.updateShowDeleteConfirmation(note) }) { if (state.navigateToEdit) { - mainNavController.navigate( + navController.navigate( NotebookRoute.EditNotebook( noteId = note.id, highlightedTextStartOffset = note.highlightedText.range.startOffset, @@ -189,7 +201,7 @@ fun NotebookScreen( ) ) } else { - mainNavController.navigate( + navController.navigate( MainNavigationRoute.ModuleItemSequence( courseId = note.courseId, moduleItemAssetType = note.objectType.value, @@ -368,14 +380,18 @@ private fun NoteContent( Text( text = note.updatedAt.localisedFormat("MMM d, yyyy"), style = HorizonTypography.labelMediumBold, - color = HorizonColors.Text.timestamp() + color = HorizonColors.Text.timestamp(), + maxLines = 1, + overflow = TextOverflow.Ellipsis ) if (courseName != null) { Text( text = courseName, style = HorizonTypography.labelMediumBold, - color = HorizonColors.Text.timestamp() + color = HorizonColors.Text.timestamp(), + maxLines = 1, + overflow = TextOverflow.Ellipsis ) } } diff --git a/libs/horizon/src/test/java/com/instructure/horizon/features/notebook/NotebookRepositoryTest.kt b/libs/horizon/src/test/java/com/instructure/horizon/features/notebook/NotebookRepositoryTest.kt index e73204657d..7e260c291f 100644 --- a/libs/horizon/src/test/java/com/instructure/horizon/features/notebook/NotebookRepositoryTest.kt +++ b/libs/horizon/src/test/java/com/instructure/horizon/features/notebook/NotebookRepositoryTest.kt @@ -194,6 +194,31 @@ class NotebookRepositoryTest { coVerify { horizonGetCoursesManager.getCoursesWithProgress(userId = 123L, forceNetwork = false) } } + @Test + fun `Test deleteNote calls redwood API with noteId`() = runTest { + coEvery { redwoodApiManager.deleteNote(any()) } returns Unit + + getRepository().deleteNote("note123") + + coVerify(exactly = 1) { redwoodApiManager.deleteNote("note123") } + } + + @Test(expected = Exception::class) + fun `Test deleteNote propagates API errors`() = runTest { + coEvery { redwoodApiManager.deleteNote(any()) } throws Exception("Delete failed") + + getRepository().deleteNote("note123") + } + + @Test + fun `Test deleteNote with empty noteId calls API`() = runTest { + coEvery { redwoodApiManager.deleteNote(any()) } returns Unit + + getRepository().deleteNote("") + + coVerify(exactly = 1) { redwoodApiManager.deleteNote("") } + } + private fun getRepository(): NotebookRepository { return NotebookRepository(redwoodApiManager, horizonGetCoursesManager, apiPrefs) } diff --git a/libs/horizon/src/test/java/com/instructure/horizon/features/notebook/NotebookViewModelTest.kt b/libs/horizon/src/test/java/com/instructure/horizon/features/notebook/NotebookViewModelTest.kt index 6d4f3e107e..b8e3076fff 100644 --- a/libs/horizon/src/test/java/com/instructure/horizon/features/notebook/NotebookViewModelTest.kt +++ b/libs/horizon/src/test/java/com/instructure/horizon/features/notebook/NotebookViewModelTest.kt @@ -239,6 +239,78 @@ class NotebookViewModelTest { assertFalse(viewModel.uiState.value.showCourseFilter) } + @Test + fun `Test updateShowDeleteConfirmation updates state correctly`() = runTest { + val viewModel = getViewModel() + val testNote = viewModel.uiState.value.notes.first() + + viewModel.uiState.value.updateShowDeleteConfirmation(testNote) + + assertEquals(testNote, viewModel.uiState.value.showDeleteConfirmationForNote) + } + + @Test + fun `Test updateShowDeleteConfirmation with null clears confirmation`() = runTest { + val viewModel = getViewModel() + val testNote = viewModel.uiState.value.notes.first() + + viewModel.uiState.value.updateShowDeleteConfirmation(testNote) + viewModel.uiState.value.updateShowDeleteConfirmation(null) + + assertNull(viewModel.uiState.value.showDeleteConfirmationForNote) + } + + @Test + fun `Test deleteNote removes note from list after successful deletion`() = runTest { + coEvery { repository.deleteNote(any()) } returns Unit + val viewModel = getViewModel() + val initialNotesCount = viewModel.uiState.value.notes.size + val noteToDelete = viewModel.uiState.value.notes.first() + + viewModel.uiState.value.deleteNote(noteToDelete) + + assertEquals(initialNotesCount - 1, viewModel.uiState.value.notes.size) + assertFalse(viewModel.uiState.value.notes.contains(noteToDelete)) + assertNull(viewModel.uiState.value.deleteLoadingNote) + coVerify(exactly = 1) { repository.deleteNote(noteToDelete.id) } + } + + @Test + fun `Test deleteNote clears loading state on error`() = runTest { + coEvery { repository.deleteNote(any()) } throws Exception("Delete failed") + val viewModel = getViewModel() + val initialNotesCount = viewModel.uiState.value.notes.size + val noteToDelete = viewModel.uiState.value.notes.first() + + viewModel.uiState.value.deleteNote(noteToDelete) + + assertEquals(initialNotesCount, viewModel.uiState.value.notes.size) + assertTrue(viewModel.uiState.value.notes.contains(noteToDelete)) + assertNull(viewModel.uiState.value.deleteLoadingNote) + } + + @Test + fun `Test deleteNote with null note does nothing`() = runTest { + val viewModel = getViewModel() + val initialNotesCount = viewModel.uiState.value.notes.size + + viewModel.uiState.value.deleteNote(null) + + assertEquals(initialNotesCount, viewModel.uiState.value.notes.size) + coVerify(exactly = 0) { repository.deleteNote(any()) } + } + + @Test + fun `Test deleteNote calls repository with correct noteId`() = runTest { + coEvery { repository.deleteNote(any()) } returns Unit + val viewModel = getViewModel() + val noteToDelete = viewModel.uiState.value.notes.first() + + viewModel.uiState.value.deleteNote(noteToDelete) + + coVerify(exactly = 1) { repository.deleteNote(noteToDelete.id) } + } + private fun getViewModel(): NotebookViewModel { return NotebookViewModel(context, repository, savedStateHandle) } From 95603f3ce5f03aa989da71f29ce30690bb6c4958 Mon Sep 17 00:00:00 2001 From: domonkosadam Date: Wed, 26 Nov 2025 15:34:22 +0100 Subject: [PATCH 28/33] Fix top app bar style --- .../horizon/features/notebook/NotebookScreen.kt | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt index b17aa0f936..0df9e5fdd5 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt @@ -124,10 +124,18 @@ fun NotebookScreen( modifier = Modifier.background(HorizonColors.Surface.pagePrimary()), headerContent = { if (state.showTopBar) { - NotebookAppBar( - navigateBack = { navController.popBackStack() }, - centeredTitle = true - ) + if (state.showCourseFilter) { + NotebookAppBar( + navigateBack = { navController.popBackStack() }, + centeredTitle = true + ) + } else { + NotebookAppBar( + onClose = { navController.popBackStack() }, + centeredTitle = false + ) + } + } }, bodyContent = { From 8d3ee76a99d1b7f38b871f63ab39bb55258fa47e Mon Sep 17 00:00:00 2001 From: domonkosadam Date: Thu, 27 Nov 2025 11:01:17 +0100 Subject: [PATCH 29/33] a11y improvements --- .../horizon/features/notebook/NotebookScreen.kt | 13 +++++++++++-- .../horizon/horizonui/organisms/Modal.kt | 2 ++ libs/horizon/src/main/res/values/strings.xml | 1 + 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt index 0df9e5fdd5..ea8cecfccc 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt @@ -43,6 +43,8 @@ import androidx.compose.ui.draw.clip import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.res.colorResource import androidx.compose.ui.res.stringResource +import androidx.compose.ui.semantics.clearAndSetSemantics +import androidx.compose.ui.semantics.contentDescription import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.unit.dp @@ -350,13 +352,17 @@ private fun NoteContent( .fillMaxWidth() .padding(24.dp) ) { + val typeName = stringResource(note.type.labelRes) NotebookTypeSelect( note.type, verticalPadding = 2.dp, onSelect = {}, showIcons = true, enabled = false, - showAllOption = false + showAllOption = false, + modifier = Modifier.clearAndSetSemantics { + contentDescription = typeName + } ) HorizonSpace(SpaceSize.SPACE_16) @@ -383,7 +389,9 @@ private fun NoteContent( Row { Column( - modifier = Modifier.weight(1f).align(Alignment.CenterVertically) + modifier = Modifier + .weight(1f) + .align(Alignment.CenterVertically) ){ Text( text = note.updatedAt.localisedFormat("MMM d, yyyy"), @@ -406,6 +414,7 @@ private fun NoteContent( LoadingIconButton( iconRes = R.drawable.delete, + contentDescription = stringResource(R.string.a11y_notebookDeleteNoteButtonContentDescription), color = IconButtonColor.InverseDanger, size = IconButtonSize.SMALL, onClick = { onDeleteClick() }, diff --git a/libs/horizon/src/main/java/com/instructure/horizon/horizonui/organisms/Modal.kt b/libs/horizon/src/main/java/com/instructure/horizon/horizonui/organisms/Modal.kt index 1457ee3996..160c91134f 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/horizonui/organisms/Modal.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/horizonui/organisms/Modal.kt @@ -31,6 +31,7 @@ import androidx.compose.runtime.Composable import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.platform.LocalContext +import androidx.compose.ui.res.stringResource import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.unit.dp import androidx.compose.ui.window.Dialog @@ -104,6 +105,7 @@ private fun DialogHeader( Text(text = title, style = HorizonTypography.h3, modifier = Modifier.weight(1f)) IconButton( iconRes = R.drawable.close, + contentDescription = stringResource(R.string.a11y_close), size = IconButtonSize.SMALL, color = IconButtonColor.Inverse, elevation = HorizonElevation.level4, diff --git a/libs/horizon/src/main/res/values/strings.xml b/libs/horizon/src/main/res/values/strings.xml index 8341112cbd..8e87a2d57e 100644 --- a/libs/horizon/src/main/res/values/strings.xml +++ b/libs/horizon/src/main/res/values/strings.xml @@ -478,4 +478,5 @@ Are you sure you want to discard your changes? Exit Cancel + Delete note \ No newline at end of file From 9a5140926e05dbf70edaf9eed524b3dff4f13557 Mon Sep 17 00:00:00 2001 From: domonkosadam Date: Thu, 27 Nov 2025 13:48:15 +0100 Subject: [PATCH 30/33] Implement navigation animations --- .../notebook/navigation/NotebookNavigation.kt | 70 ++++++++++++++++++- .../animation/NavigationTransation.kt | 36 +++++++++- .../horizon/navigation/HorizonNavigation.kt | 55 ++++++++++++++- 3 files changed, 157 insertions(+), 4 deletions(-) diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/navigation/NotebookNavigation.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/navigation/NotebookNavigation.kt index ef45e0b86d..804ecf3c8a 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/navigation/NotebookNavigation.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/navigation/NotebookNavigation.kt @@ -16,9 +16,11 @@ */ package com.instructure.horizon.features.notebook.navigation +import androidx.compose.animation.AnimatedContentTransitionScope import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue import androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel +import androidx.navigation.NavBackStackEntry import androidx.navigation.NavGraphBuilder import androidx.navigation.NavHostController import androidx.navigation.NavType @@ -32,9 +34,16 @@ import com.instructure.horizon.features.notebook.addedit.add.AddNoteViewModel import com.instructure.horizon.features.notebook.addedit.edit.EditNoteViewModel import com.instructure.horizon.horizonui.animation.enterTransition import com.instructure.horizon.horizonui.animation.exitTransition +import com.instructure.horizon.horizonui.animation.mainEnterTransition +import com.instructure.horizon.horizonui.animation.mainExitTransition +import com.instructure.horizon.horizonui.animation.overlayEnterTransition +import com.instructure.horizon.horizonui.animation.overlayExitTransition +import com.instructure.horizon.horizonui.animation.overlayPopEnterTransition +import com.instructure.horizon.horizonui.animation.overlayPopExitTransition import com.instructure.horizon.horizonui.animation.popEnterTransition import com.instructure.horizon.horizonui.animation.popExitTransition import com.instructure.horizon.navigation.MainNavigationRoute +import com.instructure.pandautils.utils.orDefault fun NavGraphBuilder.notebookNavigation( navController: NavHostController, @@ -50,6 +59,10 @@ fun NavGraphBuilder.notebookNavigation( ) { composable( route = NotebookRoute.Notebook.route, + enterTransition = { overlayEnterTransition(::isOverlayEnterTransition) }, + exitTransition = { overlayExitTransition(::isOverlayExitTransition) }, + popEnterTransition = { overlayPopEnterTransition(::isOverlayPopEnterTransition) }, + popExitTransition = { overlayPopExitTransition(::isOverlayPopExitTransition) }, arguments = listOf( navArgument(NotebookRoute.Notebook.COURSE_ID) { type = NavType.StringType @@ -80,15 +93,68 @@ fun NavGraphBuilder.notebookNavigation( val viewModel = hiltViewModel() NotebookScreen(navController, viewModel) } - composable { + composable( + enterTransition = { mainEnterTransition }, + exitTransition = { mainExitTransition }, + popEnterTransition = { mainEnterTransition }, + popExitTransition = { mainExitTransition }, + ) { val viewModel = hiltViewModel() val uiState by viewModel.uiState.collectAsState() AddEditNoteScreen(navController, uiState, onShowSnackbar) } - composable { + composable( + enterTransition = { mainEnterTransition }, + exitTransition = { mainExitTransition }, + popEnterTransition = { mainEnterTransition }, + popExitTransition = { mainExitTransition }, + ) { val viewModel = hiltViewModel() val uiState by viewModel.uiState.collectAsState() AddEditNoteScreen(navController, uiState, onShowSnackbar) } } +} + +private fun AnimatedContentTransitionScope.isOverlayEnterTransition(): Boolean { + val initialRoute = initialState.destination.route + val targetRoute = targetState.destination.route + + val targetIsNotebook = targetRoute?.startsWith(NotebookRoute.Notebook.route).orDefault() + val isFromEditNotebook = initialRoute?.startsWith(NotebookRoute.EditNotebook::class.java.name.replace("$", ".")).orDefault() + val isFromModuleItemSequence = initialRoute?.startsWith(MainNavigationRoute.ModuleItemSequence::class.java.name.replace("$", ".")).orDefault() + + return targetIsNotebook && (isFromEditNotebook || isFromModuleItemSequence) +} + +private fun AnimatedContentTransitionScope.isOverlayExitTransition(): Boolean { + val initialRoute = initialState.destination.route + val targetRoute = targetState.destination.route + + val isFromNotebook = initialRoute?.startsWith(NotebookRoute.Notebook.route).orDefault() + val targetIsEditNotebook = targetRoute?.startsWith(NotebookRoute.EditNotebook::class.java.name.replace("$", ".")).orDefault() + + return isFromNotebook && targetIsEditNotebook +} + +private fun AnimatedContentTransitionScope.isOverlayPopEnterTransition(): Boolean { + val initialRoute = initialState.destination.route + val targetRoute = targetState.destination.route + + val targetIsNotebook = targetRoute?.startsWith(NotebookRoute.Notebook.route).orDefault() + val isFromEditNotebook = initialRoute?.startsWith(NotebookRoute.EditNotebook::class.java.name.replace("$", ".")).orDefault() + val isFromModuleItemSequence = initialRoute?.startsWith(MainNavigationRoute.ModuleItemSequence::class.java.name.replace("$", ".")).orDefault() + + return targetIsNotebook && isFromEditNotebook && !isFromModuleItemSequence +} + +private fun AnimatedContentTransitionScope.isOverlayPopExitTransition(): Boolean { + val initialRoute = initialState.destination.route + val targetRoute = targetState.destination.route + + val isFromNotebook = initialRoute?.startsWith(NotebookRoute.Notebook.route).orDefault() + val targetIsModuleItemSequence = targetRoute?.startsWith(MainNavigationRoute.ModuleItemSequence::class.java.name.replace("$", ".")).orDefault() + val targetIsEditNotebook = targetRoute?.startsWith(NotebookRoute.EditNotebook::class.java.name.replace("$", ".")).orDefault() + + return isFromNotebook && (targetIsModuleItemSequence || targetIsEditNotebook) } \ No newline at end of file diff --git a/libs/horizon/src/main/java/com/instructure/horizon/horizonui/animation/NavigationTransation.kt b/libs/horizon/src/main/java/com/instructure/horizon/horizonui/animation/NavigationTransation.kt index ef92690f59..aaea51fd44 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/horizonui/animation/NavigationTransation.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/horizonui/animation/NavigationTransation.kt @@ -16,6 +16,8 @@ */ package com.instructure.horizon.horizonui.animation +import androidx.compose.animation.EnterTransition +import androidx.compose.animation.ExitTransition import androidx.compose.animation.fadeIn import androidx.compose.animation.fadeOut import androidx.compose.animation.scaleIn @@ -42,4 +44,36 @@ val mainEnterTransition = fadeIn(initialAlpha = animatedAlpha) + scaleIn(initialScale = animatedScale) val mainExitTransition = fadeOut(targetAlpha = animatedAlpha) + - scaleOut(targetScale = animatedScale) \ No newline at end of file + scaleOut(targetScale = animatedScale) + +fun overlayEnterTransition(isOverlayTransition: () -> Boolean = { true }): EnterTransition { + return if (isOverlayTransition()) { + mainEnterTransition + } else { + enterTransition + } +} + +fun overlayExitTransition(isOverlayTransition: () -> Boolean = { true }): ExitTransition { + return if (isOverlayTransition()) { + ExitTransition.None + } else { + exitTransition + } +} + +fun overlayPopEnterTransition(isOverlayTransition: () -> Boolean = { true }): EnterTransition { + return if (isOverlayTransition()) { + EnterTransition.None + } else { + popEnterTransition + } +} + +fun overlayPopExitTransition(isOverlayTransition: () -> Boolean = { true }): ExitTransition { + return if (isOverlayTransition()) { + ExitTransition.None + } else { + popExitTransition + } +} \ No newline at end of file diff --git a/libs/horizon/src/main/java/com/instructure/horizon/navigation/HorizonNavigation.kt b/libs/horizon/src/main/java/com/instructure/horizon/navigation/HorizonNavigation.kt index 3e26a88b00..91e99ac1a0 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/navigation/HorizonNavigation.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/navigation/HorizonNavigation.kt @@ -15,6 +15,7 @@ */ package com.instructure.horizon.navigation +import androidx.compose.animation.AnimatedContentTransitionScope import androidx.compose.foundation.layout.padding import androidx.compose.material3.Scaffold import androidx.compose.material3.SnackbarHost @@ -28,6 +29,7 @@ import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.ui.Modifier import androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel +import androidx.navigation.NavBackStackEntry import androidx.navigation.NavHostController import androidx.navigation.NavType import androidx.navigation.compose.NavHost @@ -40,11 +42,16 @@ import com.instructure.horizon.features.home.HomeViewModel import com.instructure.horizon.features.inbox.navigation.horizonInboxNavigation import com.instructure.horizon.features.moduleitemsequence.ModuleItemSequenceScreen import com.instructure.horizon.features.moduleitemsequence.ModuleItemSequenceViewModel +import com.instructure.horizon.features.notebook.navigation.NotebookRoute import com.instructure.horizon.features.notebook.navigation.notebookNavigation import com.instructure.horizon.features.notification.NotificationScreen import com.instructure.horizon.features.notification.NotificationViewModel import com.instructure.horizon.horizonui.animation.enterTransition import com.instructure.horizon.horizonui.animation.exitTransition +import com.instructure.horizon.horizonui.animation.overlayEnterTransition +import com.instructure.horizon.horizonui.animation.overlayExitTransition +import com.instructure.horizon.horizonui.animation.overlayPopEnterTransition +import com.instructure.horizon.horizonui.animation.overlayPopExitTransition import com.instructure.horizon.horizonui.animation.popEnterTransition import com.instructure.horizon.horizonui.animation.popExitTransition import com.instructure.horizon.horizonui.foundation.HorizonColors @@ -52,6 +59,7 @@ import com.instructure.horizon.navigation.MainNavigationRoute.Companion.ASSIGNME import com.instructure.horizon.navigation.MainNavigationRoute.Companion.COURSE_ID import com.instructure.horizon.navigation.MainNavigationRoute.Companion.PAGE_ID import com.instructure.horizon.navigation.MainNavigationRoute.Companion.QUIZ_ID +import com.instructure.pandautils.utils.orDefault import kotlinx.coroutines.launch import kotlinx.serialization.Serializable @@ -114,7 +122,12 @@ fun HorizonNavigation(navController: NavHostController, modifier: Modifier = Mod composable(MainNavigationRoute.Home.route) { HomeScreen(navController, hiltViewModel()) } - composable { + composable( + enterTransition = { overlayEnterTransition(::isOverlayEnterTransition) }, + exitTransition = { overlayExitTransition(::isOverlayExitTransition) }, + popEnterTransition = { overlayPopEnterTransition(::isOverlayPopEnterTransition) }, + popExitTransition = { overlayPopExitTransition(::isOverlayPopExitTransition) } + ) { val viewModel = hiltViewModel() val uiState by viewModel.uiState.collectAsState() ModuleItemSequenceScreen(navController, uiState) @@ -235,4 +248,44 @@ fun HorizonNavigation(navController: NavHostController, modifier: Modifier = Mod } } } +} + +private fun AnimatedContentTransitionScope.isOverlayEnterTransition(): Boolean { + val initialRoute = initialState.destination.route + val targetRoute = targetState.destination.route + + val targetIsModuleItemSequence = targetRoute?.startsWith(MainNavigationRoute.ModuleItemSequence::class.java.name.replace("$", ".")).orDefault() + val isFromNotebook = initialRoute?.startsWith(NotebookRoute.Notebook.route).orDefault() + + return targetIsModuleItemSequence && !isFromNotebook +} + +private fun AnimatedContentTransitionScope.isOverlayExitTransition(): Boolean { + val initialRoute = initialState.destination.route + val targetRoute = targetState.destination.route + + val isFromModuleItemSequence = initialRoute?.startsWith(MainNavigationRoute.ModuleItemSequence::class.java.name.replace("$", ".")).orDefault() + val targetIsNotebook = targetRoute?.startsWith(NotebookRoute.Notebook.route).orDefault() + + return isFromModuleItemSequence && targetIsNotebook +} + +private fun AnimatedContentTransitionScope.isOverlayPopEnterTransition(): Boolean { + val initialRoute = initialState.destination.route + val targetRoute = targetState.destination.route + + val targetIsModuleItemSequence = targetRoute?.startsWith(MainNavigationRoute.ModuleItemSequence::class.java.name.replace("$", ".")).orDefault() + val isFromNotebook = initialRoute?.startsWith(NotebookRoute.Notebook.route).orDefault() + + return targetIsModuleItemSequence && isFromNotebook +} + +private fun AnimatedContentTransitionScope.isOverlayPopExitTransition(): Boolean { + val initialRoute = initialState.destination.route + val targetRoute = targetState.destination.route + + val isFromModuleItemSequence = initialRoute?.startsWith(MainNavigationRoute.ModuleItemSequence::class.java.name.replace("$", ".")).orDefault() + val targetIsNotebook = targetRoute?.startsWith(NotebookRoute.Notebook.route).orDefault() + + return isFromModuleItemSequence && !targetIsNotebook } \ No newline at end of file From fd37e21cc1e29f949cf2181b0d0bef66512c8bbc Mon Sep 17 00:00:00 2001 From: domonkosadam Date: Thu, 27 Nov 2025 13:59:14 +0100 Subject: [PATCH 31/33] Fix animations --- .../horizon/navigation/HorizonNavigation.kt | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/libs/horizon/src/main/java/com/instructure/horizon/navigation/HorizonNavigation.kt b/libs/horizon/src/main/java/com/instructure/horizon/navigation/HorizonNavigation.kt index 91e99ac1a0..f9739665ad 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/navigation/HorizonNavigation.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/navigation/HorizonNavigation.kt @@ -256,8 +256,10 @@ private fun AnimatedContentTransitionScope.isOverlayEnterTran val targetIsModuleItemSequence = targetRoute?.startsWith(MainNavigationRoute.ModuleItemSequence::class.java.name.replace("$", ".")).orDefault() val isFromNotebook = initialRoute?.startsWith(NotebookRoute.Notebook.route).orDefault() + val isFromAddNote = initialRoute?.startsWith(NotebookRoute.AddNotebook::class.java.name.replace("$", ".")).orDefault() + val isFromEditNote = initialRoute?.startsWith(NotebookRoute.EditNotebook::class.java.name.replace("$", ".")).orDefault() - return targetIsModuleItemSequence && !isFromNotebook + return targetIsModuleItemSequence && !(isFromNotebook || isFromAddNote || isFromEditNote) } private fun AnimatedContentTransitionScope.isOverlayExitTransition(): Boolean { @@ -266,8 +268,10 @@ private fun AnimatedContentTransitionScope.isOverlayExitTrans val isFromModuleItemSequence = initialRoute?.startsWith(MainNavigationRoute.ModuleItemSequence::class.java.name.replace("$", ".")).orDefault() val targetIsNotebook = targetRoute?.startsWith(NotebookRoute.Notebook.route).orDefault() + val targetIsAddNote = targetRoute?.startsWith(NotebookRoute.AddNotebook::class.java.name.replace("$", ".")).orDefault() + val targetIsEditNote = targetRoute?.startsWith(NotebookRoute.EditNotebook::class.java.name.replace("$", ".")).orDefault() - return isFromModuleItemSequence && targetIsNotebook + return isFromModuleItemSequence && (targetIsNotebook || targetIsAddNote || targetIsEditNote) } private fun AnimatedContentTransitionScope.isOverlayPopEnterTransition(): Boolean { @@ -276,8 +280,10 @@ private fun AnimatedContentTransitionScope.isOverlayPopEnterT val targetIsModuleItemSequence = targetRoute?.startsWith(MainNavigationRoute.ModuleItemSequence::class.java.name.replace("$", ".")).orDefault() val isFromNotebook = initialRoute?.startsWith(NotebookRoute.Notebook.route).orDefault() + val isFromAddNote = initialRoute?.startsWith(NotebookRoute.AddNotebook::class.java.name.replace("$", ".")).orDefault() + val isFromEditNote = initialRoute?.startsWith(NotebookRoute.EditNotebook::class.java.name.replace("$", ".")).orDefault() - return targetIsModuleItemSequence && isFromNotebook + return targetIsModuleItemSequence && (isFromNotebook || isFromAddNote || isFromEditNote) } private fun AnimatedContentTransitionScope.isOverlayPopExitTransition(): Boolean { @@ -286,6 +292,8 @@ private fun AnimatedContentTransitionScope.isOverlayPopExitTr val isFromModuleItemSequence = initialRoute?.startsWith(MainNavigationRoute.ModuleItemSequence::class.java.name.replace("$", ".")).orDefault() val targetIsNotebook = targetRoute?.startsWith(NotebookRoute.Notebook.route).orDefault() + val targetIsAddNote = targetRoute?.startsWith(NotebookRoute.AddNotebook::class.java.name.replace("$", ".")).orDefault() + val targetIsEditNote = targetRoute?.startsWith(NotebookRoute.EditNotebook::class.java.name.replace("$", ".")).orDefault() - return isFromModuleItemSequence && !targetIsNotebook + return isFromModuleItemSequence && !(targetIsNotebook || targetIsAddNote || targetIsEditNote) } \ No newline at end of file From 4d0da041e64afbfcb1aaa6a21faa6ec7401892fa Mon Sep 17 00:00:00 2001 From: domonkosadam Date: Thu, 27 Nov 2025 15:18:22 +0100 Subject: [PATCH 32/33] Refactor navigation transition handling --- .../account/navigation/AccountNavigation.kt | 24 +---- .../horizon/features/home/HomeNavigation.kt | 15 +-- .../navigation/HorizonInboxNavigation.kt | 38 +------ .../notebook/navigation/NotebookNavigation.kt | 78 ++------------ .../animation/NavigationTransation.kt | 100 ++++++++++++------ .../horizon/navigation/HorizonNavigation.kt | 71 ++----------- .../HorizonNavigationTransitionRules.kt | 90 ++++++++++++++++ 7 files changed, 192 insertions(+), 224 deletions(-) create mode 100644 libs/horizon/src/main/java/com/instructure/horizon/navigation/HorizonNavigationTransitionRules.kt diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/account/navigation/AccountNavigation.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/account/navigation/AccountNavigation.kt index 02987a6c15..1c6930943f 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/account/navigation/AccountNavigation.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/account/navigation/AccountNavigation.kt @@ -16,11 +16,9 @@ */ package com.instructure.horizon.features.account.navigation -import androidx.compose.animation.AnimatedContentTransitionScope import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue import androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel -import androidx.navigation.NavBackStackEntry import androidx.navigation.NavGraphBuilder import androidx.navigation.NavHostController import androidx.navigation.compose.composable @@ -38,10 +36,9 @@ import com.instructure.horizon.features.account.profile.AccountProfileScreen import com.instructure.horizon.features.account.profile.AccountProfileViewModel import com.instructure.horizon.features.account.reportabug.ReportABugWebView import com.instructure.horizon.features.home.HomeNavigationRoute +import com.instructure.horizon.horizonui.animation.NavigationTransitionAnimation import com.instructure.horizon.horizonui.animation.enterTransition import com.instructure.horizon.horizonui.animation.exitTransition -import com.instructure.horizon.horizonui.animation.mainEnterTransition -import com.instructure.horizon.horizonui.animation.mainExitTransition import com.instructure.horizon.horizonui.animation.popEnterTransition import com.instructure.horizon.horizonui.animation.popExitTransition @@ -51,10 +48,10 @@ fun NavGraphBuilder.accountNavigation( navigation( route = HomeNavigationRoute.Account.route, startDestination = AccountRoute.Account.route, - enterTransition = { if (isBottomNavDestination()) mainEnterTransition else enterTransition }, - exitTransition = { if (isBottomNavDestination()) mainExitTransition else exitTransition }, - popEnterTransition = { if (isBottomNavDestination()) mainEnterTransition else popEnterTransition }, - popExitTransition = { if (isBottomNavDestination()) mainExitTransition else popExitTransition }, + enterTransition = { enterTransition(NavigationTransitionAnimation.SCALE) }, + exitTransition = { exitTransition(NavigationTransitionAnimation.SCALE) }, + popEnterTransition = { popEnterTransition(NavigationTransitionAnimation.SCALE) }, + popExitTransition = { popExitTransition(NavigationTransitionAnimation.SCALE) }, ) { composable( route = AccountRoute.Account.route, @@ -96,15 +93,4 @@ fun NavGraphBuilder.accountNavigation( ReportABugWebView(navController) } } -} - -private fun AnimatedContentTransitionScope.isBottomNavDestination(): Boolean { - val sourceRoute = this.initialState.destination.route ?: return false - val destinationRoute = this.targetState.destination.route ?: return false - return sourceRoute.contains(HomeNavigationRoute.Learn.route) - || sourceRoute.contains(HomeNavigationRoute.Dashboard.route) - || sourceRoute.contains(HomeNavigationRoute.Skillspace.route) - || destinationRoute.contains(HomeNavigationRoute.Learn.route) - || destinationRoute.contains(HomeNavigationRoute.Dashboard.route) - || destinationRoute.contains(HomeNavigationRoute.Skillspace.route) } \ No newline at end of file diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/home/HomeNavigation.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/home/HomeNavigation.kt index 8d383b4193..39b6f9a5df 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/home/HomeNavigation.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/home/HomeNavigation.kt @@ -34,8 +34,11 @@ import com.instructure.horizon.features.learn.LearnScreen import com.instructure.horizon.features.learn.LearnViewModel import com.instructure.horizon.features.skillspace.SkillspaceScreen import com.instructure.horizon.features.skillspace.SkillspaceViewModel -import com.instructure.horizon.horizonui.animation.mainEnterTransition -import com.instructure.horizon.horizonui.animation.mainExitTransition +import com.instructure.horizon.horizonui.animation.NavigationTransitionAnimation +import com.instructure.horizon.horizonui.animation.enterTransition +import com.instructure.horizon.horizonui.animation.exitTransition +import com.instructure.horizon.horizonui.animation.popEnterTransition +import com.instructure.horizon.horizonui.animation.popExitTransition import com.instructure.horizon.horizonui.showroom.ShowroomContent import com.instructure.horizon.horizonui.showroom.ShowroomItem import com.instructure.horizon.horizonui.showroom.showroomItems @@ -64,10 +67,10 @@ sealed class HomeNavigationRoute(val route: String) { fun HomeNavigation(navController: NavHostController, mainNavController: NavHostController, modifier: Modifier = Modifier) { NavHost( navController, - enterTransition = { mainEnterTransition }, - exitTransition = { mainExitTransition }, - popEnterTransition = { mainEnterTransition }, - popExitTransition = { mainExitTransition }, + enterTransition = { enterTransition(NavigationTransitionAnimation.SCALE) }, + exitTransition = { exitTransition(NavigationTransitionAnimation.SCALE) }, + popEnterTransition = { popEnterTransition(NavigationTransitionAnimation.SCALE) }, + popExitTransition = { popExitTransition(NavigationTransitionAnimation.SCALE) }, startDestination = HomeNavigationRoute.Dashboard.route, modifier = modifier ) { composable(HomeNavigationRoute.Dashboard.route) { diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/inbox/navigation/HorizonInboxNavigation.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/inbox/navigation/HorizonInboxNavigation.kt index 66ffe49d1d..63d2f67352 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/inbox/navigation/HorizonInboxNavigation.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/inbox/navigation/HorizonInboxNavigation.kt @@ -16,12 +16,10 @@ */ package com.instructure.horizon.features.inbox.navigation -import androidx.compose.animation.AnimatedContentTransitionScope import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue import androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel -import androidx.navigation.NavBackStackEntry import androidx.navigation.NavGraphBuilder import androidx.navigation.NavHostController import androidx.navigation.compose.composable @@ -39,12 +37,9 @@ import com.instructure.horizon.features.inbox.list.HorizonInboxListScreen import com.instructure.horizon.features.inbox.list.HorizonInboxListViewModel import com.instructure.horizon.horizonui.animation.enterTransition import com.instructure.horizon.horizonui.animation.exitTransition -import com.instructure.horizon.horizonui.animation.mainEnterTransition -import com.instructure.horizon.horizonui.animation.mainExitTransition import com.instructure.horizon.horizonui.animation.popEnterTransition import com.instructure.horizon.horizonui.animation.popExitTransition import com.instructure.horizon.navigation.MainNavigationRoute -import com.instructure.pandautils.utils.orDefault fun NavGraphBuilder.horizonInboxNavigation( navController: NavHostController, @@ -55,10 +50,10 @@ fun NavGraphBuilder.horizonInboxNavigation( ) { composable( HorizonInboxRoute.InboxList.route, - enterTransition = { if (isComposeTransition()) mainEnterTransition else enterTransition }, - exitTransition = { if (isComposeTransition()) mainExitTransition else exitTransition }, - popEnterTransition = { if (isComposeTransition()) mainEnterTransition else popEnterTransition }, - popExitTransition = { if (isComposeTransition()) mainExitTransition else popExitTransition }, + enterTransition = { enterTransition() }, + exitTransition = { exitTransition() }, + popEnterTransition = { popEnterTransition() }, + popExitTransition = { popExitTransition() }, ) { val viewModel = hiltViewModel() val uiState by viewModel.uiState.collectAsState() @@ -66,10 +61,6 @@ fun NavGraphBuilder.horizonInboxNavigation( } composable( HorizonInboxRoute.InboxDetails.route, - enterTransition = { enterTransition }, - exitTransition = { exitTransition }, - popEnterTransition = { popEnterTransition }, - popExitTransition = { popExitTransition }, arguments = listOf( navArgument(HorizonInboxRoute.InboxDetails.TYPE) { type = androidx.navigation.NavType.StringType @@ -90,10 +81,6 @@ fun NavGraphBuilder.horizonInboxNavigation( } composable( HorizonInboxRoute.InboxCompose.route, - enterTransition = { mainEnterTransition }, - exitTransition = { mainExitTransition }, - popEnterTransition = { mainEnterTransition }, - popExitTransition = { mainExitTransition }, ) { val viewModel: HorizonInboxComposeViewModel = hiltViewModel() val uiState by viewModel.uiState.collectAsState() @@ -107,10 +94,6 @@ fun NavGraphBuilder.horizonInboxNavigation( // Conversation Details from deeplink composable( HorizonInboxRoute.InboxDetailsDeepLink.route, - enterTransition = { mainEnterTransition }, - exitTransition = { mainExitTransition }, - popEnterTransition = { mainEnterTransition }, - popExitTransition = { mainExitTransition }, arguments = listOf( navArgument(HorizonInboxRoute.InboxDetails.ID) { type = androidx.navigation.NavType.LongType @@ -135,10 +118,6 @@ fun NavGraphBuilder.horizonInboxNavigation( // Announcement Details from deeplink composable( HorizonInboxRoute.CourseAnnouncementDetailsDeepLink.route, - enterTransition = { mainEnterTransition }, - exitTransition = { mainExitTransition }, - popEnterTransition = { mainEnterTransition }, - popExitTransition = { mainExitTransition }, arguments = listOf( navArgument(HorizonInboxRoute.InboxDetails.ID) { type = androidx.navigation.NavType.LongType @@ -167,13 +146,4 @@ fun NavGraphBuilder.horizonInboxNavigation( } } } -} - -private fun AnimatedContentTransitionScope.isComposeTransition(): Boolean { - return this.targetState.destination.route - ?.startsWith(HorizonInboxRoute.InboxCompose.route) - .orDefault() || - this.initialState.destination.route - ?.startsWith(HorizonInboxRoute.InboxCompose.route) - .orDefault() } \ No newline at end of file diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/navigation/NotebookNavigation.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/navigation/NotebookNavigation.kt index 804ecf3c8a..70eefb369b 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/navigation/NotebookNavigation.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/navigation/NotebookNavigation.kt @@ -16,11 +16,9 @@ */ package com.instructure.horizon.features.notebook.navigation -import androidx.compose.animation.AnimatedContentTransitionScope import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue import androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel -import androidx.navigation.NavBackStackEntry import androidx.navigation.NavGraphBuilder import androidx.navigation.NavHostController import androidx.navigation.NavType @@ -34,16 +32,9 @@ import com.instructure.horizon.features.notebook.addedit.add.AddNoteViewModel import com.instructure.horizon.features.notebook.addedit.edit.EditNoteViewModel import com.instructure.horizon.horizonui.animation.enterTransition import com.instructure.horizon.horizonui.animation.exitTransition -import com.instructure.horizon.horizonui.animation.mainEnterTransition -import com.instructure.horizon.horizonui.animation.mainExitTransition -import com.instructure.horizon.horizonui.animation.overlayEnterTransition -import com.instructure.horizon.horizonui.animation.overlayExitTransition -import com.instructure.horizon.horizonui.animation.overlayPopEnterTransition -import com.instructure.horizon.horizonui.animation.overlayPopExitTransition import com.instructure.horizon.horizonui.animation.popEnterTransition import com.instructure.horizon.horizonui.animation.popExitTransition import com.instructure.horizon.navigation.MainNavigationRoute -import com.instructure.pandautils.utils.orDefault fun NavGraphBuilder.notebookNavigation( navController: NavHostController, @@ -52,17 +43,13 @@ fun NavGraphBuilder.notebookNavigation( navigation( route = MainNavigationRoute.Notebook.route, startDestination = NotebookRoute.Notebook.route, - enterTransition = { enterTransition }, - exitTransition = { exitTransition }, - popEnterTransition = { popEnterTransition }, - popExitTransition = { popExitTransition }, + enterTransition = { enterTransition() }, + exitTransition = { exitTransition() }, + popEnterTransition = { popEnterTransition() }, + popExitTransition = { popExitTransition() }, ) { composable( route = NotebookRoute.Notebook.route, - enterTransition = { overlayEnterTransition(::isOverlayEnterTransition) }, - exitTransition = { overlayExitTransition(::isOverlayExitTransition) }, - popEnterTransition = { overlayPopEnterTransition(::isOverlayPopEnterTransition) }, - popExitTransition = { overlayPopExitTransition(::isOverlayPopExitTransition) }, arguments = listOf( navArgument(NotebookRoute.Notebook.COURSE_ID) { type = NavType.StringType @@ -93,68 +80,15 @@ fun NavGraphBuilder.notebookNavigation( val viewModel = hiltViewModel() NotebookScreen(navController, viewModel) } - composable( - enterTransition = { mainEnterTransition }, - exitTransition = { mainExitTransition }, - popEnterTransition = { mainEnterTransition }, - popExitTransition = { mainExitTransition }, - ) { + composable { val viewModel = hiltViewModel() val uiState by viewModel.uiState.collectAsState() AddEditNoteScreen(navController, uiState, onShowSnackbar) } - composable( - enterTransition = { mainEnterTransition }, - exitTransition = { mainExitTransition }, - popEnterTransition = { mainEnterTransition }, - popExitTransition = { mainExitTransition }, - ) { + composable { val viewModel = hiltViewModel() val uiState by viewModel.uiState.collectAsState() AddEditNoteScreen(navController, uiState, onShowSnackbar) } } } - -private fun AnimatedContentTransitionScope.isOverlayEnterTransition(): Boolean { - val initialRoute = initialState.destination.route - val targetRoute = targetState.destination.route - - val targetIsNotebook = targetRoute?.startsWith(NotebookRoute.Notebook.route).orDefault() - val isFromEditNotebook = initialRoute?.startsWith(NotebookRoute.EditNotebook::class.java.name.replace("$", ".")).orDefault() - val isFromModuleItemSequence = initialRoute?.startsWith(MainNavigationRoute.ModuleItemSequence::class.java.name.replace("$", ".")).orDefault() - - return targetIsNotebook && (isFromEditNotebook || isFromModuleItemSequence) -} - -private fun AnimatedContentTransitionScope.isOverlayExitTransition(): Boolean { - val initialRoute = initialState.destination.route - val targetRoute = targetState.destination.route - - val isFromNotebook = initialRoute?.startsWith(NotebookRoute.Notebook.route).orDefault() - val targetIsEditNotebook = targetRoute?.startsWith(NotebookRoute.EditNotebook::class.java.name.replace("$", ".")).orDefault() - - return isFromNotebook && targetIsEditNotebook -} - -private fun AnimatedContentTransitionScope.isOverlayPopEnterTransition(): Boolean { - val initialRoute = initialState.destination.route - val targetRoute = targetState.destination.route - - val targetIsNotebook = targetRoute?.startsWith(NotebookRoute.Notebook.route).orDefault() - val isFromEditNotebook = initialRoute?.startsWith(NotebookRoute.EditNotebook::class.java.name.replace("$", ".")).orDefault() - val isFromModuleItemSequence = initialRoute?.startsWith(MainNavigationRoute.ModuleItemSequence::class.java.name.replace("$", ".")).orDefault() - - return targetIsNotebook && isFromEditNotebook && !isFromModuleItemSequence -} - -private fun AnimatedContentTransitionScope.isOverlayPopExitTransition(): Boolean { - val initialRoute = initialState.destination.route - val targetRoute = targetState.destination.route - - val isFromNotebook = initialRoute?.startsWith(NotebookRoute.Notebook.route).orDefault() - val targetIsModuleItemSequence = targetRoute?.startsWith(MainNavigationRoute.ModuleItemSequence::class.java.name.replace("$", ".")).orDefault() - val targetIsEditNotebook = targetRoute?.startsWith(NotebookRoute.EditNotebook::class.java.name.replace("$", ".")).orDefault() - - return isFromNotebook && (targetIsModuleItemSequence || targetIsEditNotebook) -} \ No newline at end of file diff --git a/libs/horizon/src/main/java/com/instructure/horizon/horizonui/animation/NavigationTransation.kt b/libs/horizon/src/main/java/com/instructure/horizon/horizonui/animation/NavigationTransation.kt index aaea51fd44..12413acdb3 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/horizonui/animation/NavigationTransation.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/horizonui/animation/NavigationTransation.kt @@ -16,6 +16,7 @@ */ package com.instructure.horizon.horizonui.animation +import androidx.compose.animation.AnimatedContentTransitionScope import androidx.compose.animation.EnterTransition import androidx.compose.animation.ExitTransition import androidx.compose.animation.fadeIn @@ -24,56 +25,95 @@ import androidx.compose.animation.scaleIn import androidx.compose.animation.scaleOut import androidx.compose.animation.slideInHorizontally import androidx.compose.animation.slideOutHorizontally +import androidx.navigation.NavBackStackEntry +import com.instructure.horizon.navigation.animationRules private const val animatedAlpha: Float = 0.0f private const val animatedScale: Float = 0.8f -val enterTransition = slideInHorizontally(initialOffsetX = { it }) + +private val slideEnterTransition = slideInHorizontally(initialOffsetX = { it }) + fadeIn(initialAlpha = animatedAlpha) -val exitTransition = slideOutHorizontally(targetOffsetX = { -it }) + +private val slideExitTransition = slideOutHorizontally(targetOffsetX = { -it }) + fadeOut(targetAlpha = animatedAlpha) -val popEnterTransition = slideInHorizontally(initialOffsetX = { -it }) + +private val slidePopEnterTransition = slideInHorizontally(initialOffsetX = { -it }) + fadeIn(initialAlpha = animatedAlpha) -val popExitTransition = slideOutHorizontally(targetOffsetX = { it/2 }) + +private val slidePopExitTransition = slideOutHorizontally(targetOffsetX = { it / 2 }) + fadeOut(targetAlpha = animatedAlpha) -val mainEnterTransition = fadeIn(initialAlpha = animatedAlpha) + +private val scaleEnterTransition = fadeIn(initialAlpha = animatedAlpha) + scaleIn(initialScale = animatedScale) -val mainExitTransition = fadeOut(targetAlpha = animatedAlpha) + +private val scaleExitTransition = ExitTransition.None + +private val scalePopEnterTransition = EnterTransition.None + +private val scalePopExitTransition = fadeOut(targetAlpha = animatedAlpha) + scaleOut(targetScale = animatedScale) -fun overlayEnterTransition(isOverlayTransition: () -> Boolean = { true }): EnterTransition { - return if (isOverlayTransition()) { - mainEnterTransition - } else { - enterTransition - } +enum class NavigationTransitionAnimation( + val enterTransition: EnterTransition, + val exitTransition: ExitTransition, + val popEnterTransition: EnterTransition, + val popExitTransition: ExitTransition +) { + SLIDE( + enterTransition = slideEnterTransition, + exitTransition = slideExitTransition, + popEnterTransition = slidePopEnterTransition, + popExitTransition = slidePopExitTransition + ), + SCALE( + enterTransition = scaleEnterTransition, + exitTransition = scaleExitTransition, + popEnterTransition = scalePopEnterTransition, + popExitTransition = scalePopExitTransition + ) } -fun overlayExitTransition(isOverlayTransition: () -> Boolean = { true }): ExitTransition { - return if (isOverlayTransition()) { - ExitTransition.None - } else { - exitTransition - } + +fun AnimatedContentTransitionScope.enterTransition( + defaultTransitionStyle: NavigationTransitionAnimation = NavigationTransitionAnimation.SLIDE +): EnterTransition { + val fromRoute = initialState.destination.route + val toRoute = targetState.destination.route + + return (findAnimationStyle(fromRoute, toRoute) ?: defaultTransitionStyle).enterTransition } -fun overlayPopEnterTransition(isOverlayTransition: () -> Boolean = { true }): EnterTransition { - return if (isOverlayTransition()) { - EnterTransition.None - } else { - popEnterTransition - } +fun AnimatedContentTransitionScope.exitTransition( + defaultTransitionStyle: NavigationTransitionAnimation = NavigationTransitionAnimation.SLIDE +): ExitTransition { + val fromRoute = initialState.destination.route + val toRoute = targetState.destination.route + + return (findAnimationStyle(fromRoute, toRoute) ?: defaultTransitionStyle).exitTransition +} + +fun AnimatedContentTransitionScope.popEnterTransition( + defaultTransitionStyle: NavigationTransitionAnimation = NavigationTransitionAnimation.SLIDE +): EnterTransition { + val fromRoute = initialState.destination.route + val toRoute = targetState.destination.route + + return (findAnimationStyle(toRoute, fromRoute) ?: defaultTransitionStyle).popEnterTransition +} + +fun AnimatedContentTransitionScope.popExitTransition( + defaultTransitionStyle: NavigationTransitionAnimation = NavigationTransitionAnimation.SLIDE +): ExitTransition { + val fromRoute = initialState.destination.route + val toRoute = targetState.destination.route + + return (findAnimationStyle(toRoute, fromRoute) ?: defaultTransitionStyle).popExitTransition } -fun overlayPopExitTransition(isOverlayTransition: () -> Boolean = { true }): ExitTransition { - return if (isOverlayTransition()) { - ExitTransition.None - } else { - popExitTransition - } +private fun findAnimationStyle(fromRoute: String?, toRoute: String?): NavigationTransitionAnimation? { + return animationRules.firstOrNull { rule -> + val fromMatches = rule.from == null || fromRoute?.startsWith(rule.from) == true + val toMatches = rule.to == null || toRoute?.startsWith(rule.to) == true + fromMatches && toMatches + }?.style } \ No newline at end of file diff --git a/libs/horizon/src/main/java/com/instructure/horizon/navigation/HorizonNavigation.kt b/libs/horizon/src/main/java/com/instructure/horizon/navigation/HorizonNavigation.kt index f9739665ad..7691c50df2 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/navigation/HorizonNavigation.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/navigation/HorizonNavigation.kt @@ -15,7 +15,6 @@ */ package com.instructure.horizon.navigation -import androidx.compose.animation.AnimatedContentTransitionScope import androidx.compose.foundation.layout.padding import androidx.compose.material3.Scaffold import androidx.compose.material3.SnackbarHost @@ -29,7 +28,6 @@ import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.ui.Modifier import androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel -import androidx.navigation.NavBackStackEntry import androidx.navigation.NavHostController import androidx.navigation.NavType import androidx.navigation.compose.NavHost @@ -42,16 +40,11 @@ import com.instructure.horizon.features.home.HomeViewModel import com.instructure.horizon.features.inbox.navigation.horizonInboxNavigation import com.instructure.horizon.features.moduleitemsequence.ModuleItemSequenceScreen import com.instructure.horizon.features.moduleitemsequence.ModuleItemSequenceViewModel -import com.instructure.horizon.features.notebook.navigation.NotebookRoute import com.instructure.horizon.features.notebook.navigation.notebookNavigation import com.instructure.horizon.features.notification.NotificationScreen import com.instructure.horizon.features.notification.NotificationViewModel import com.instructure.horizon.horizonui.animation.enterTransition import com.instructure.horizon.horizonui.animation.exitTransition -import com.instructure.horizon.horizonui.animation.overlayEnterTransition -import com.instructure.horizon.horizonui.animation.overlayExitTransition -import com.instructure.horizon.horizonui.animation.overlayPopEnterTransition -import com.instructure.horizon.horizonui.animation.overlayPopExitTransition import com.instructure.horizon.horizonui.animation.popEnterTransition import com.instructure.horizon.horizonui.animation.popExitTransition import com.instructure.horizon.horizonui.foundation.HorizonColors @@ -59,7 +52,6 @@ import com.instructure.horizon.navigation.MainNavigationRoute.Companion.ASSIGNME import com.instructure.horizon.navigation.MainNavigationRoute.Companion.COURSE_ID import com.instructure.horizon.navigation.MainNavigationRoute.Companion.PAGE_ID import com.instructure.horizon.navigation.MainNavigationRoute.Companion.QUIZ_ID -import com.instructure.pandautils.utils.orDefault import kotlinx.coroutines.launch import kotlinx.serialization.Serializable @@ -100,10 +92,10 @@ fun HorizonNavigation(navController: NavHostController, modifier: Modifier = Mod snackbarHost = { SnackbarHost(hostState = snackbarHostState) }, ) { innerPadding -> NavHost( - enterTransition = { enterTransition }, - exitTransition = { exitTransition }, - popEnterTransition = { popEnterTransition }, - popExitTransition = { popExitTransition }, + enterTransition = { enterTransition() }, + exitTransition = { exitTransition() }, + popEnterTransition = { popEnterTransition() }, + popExitTransition = { popExitTransition() }, modifier = modifier.padding(innerPadding), navController = navController, startDestination = MainNavigationRoute.Home.route @@ -123,10 +115,10 @@ fun HorizonNavigation(navController: NavHostController, modifier: Modifier = Mod HomeScreen(navController, hiltViewModel()) } composable( - enterTransition = { overlayEnterTransition(::isOverlayEnterTransition) }, - exitTransition = { overlayExitTransition(::isOverlayExitTransition) }, - popEnterTransition = { overlayPopEnterTransition(::isOverlayPopEnterTransition) }, - popExitTransition = { overlayPopExitTransition(::isOverlayPopExitTransition) } + enterTransition = { enterTransition() }, + exitTransition = { exitTransition() }, + popEnterTransition = { popEnterTransition() }, + popExitTransition = { popExitTransition() } ) { val viewModel = hiltViewModel() val uiState by viewModel.uiState.collectAsState() @@ -250,50 +242,3 @@ fun HorizonNavigation(navController: NavHostController, modifier: Modifier = Mod } } -private fun AnimatedContentTransitionScope.isOverlayEnterTransition(): Boolean { - val initialRoute = initialState.destination.route - val targetRoute = targetState.destination.route - - val targetIsModuleItemSequence = targetRoute?.startsWith(MainNavigationRoute.ModuleItemSequence::class.java.name.replace("$", ".")).orDefault() - val isFromNotebook = initialRoute?.startsWith(NotebookRoute.Notebook.route).orDefault() - val isFromAddNote = initialRoute?.startsWith(NotebookRoute.AddNotebook::class.java.name.replace("$", ".")).orDefault() - val isFromEditNote = initialRoute?.startsWith(NotebookRoute.EditNotebook::class.java.name.replace("$", ".")).orDefault() - - return targetIsModuleItemSequence && !(isFromNotebook || isFromAddNote || isFromEditNote) -} - -private fun AnimatedContentTransitionScope.isOverlayExitTransition(): Boolean { - val initialRoute = initialState.destination.route - val targetRoute = targetState.destination.route - - val isFromModuleItemSequence = initialRoute?.startsWith(MainNavigationRoute.ModuleItemSequence::class.java.name.replace("$", ".")).orDefault() - val targetIsNotebook = targetRoute?.startsWith(NotebookRoute.Notebook.route).orDefault() - val targetIsAddNote = targetRoute?.startsWith(NotebookRoute.AddNotebook::class.java.name.replace("$", ".")).orDefault() - val targetIsEditNote = targetRoute?.startsWith(NotebookRoute.EditNotebook::class.java.name.replace("$", ".")).orDefault() - - return isFromModuleItemSequence && (targetIsNotebook || targetIsAddNote || targetIsEditNote) -} - -private fun AnimatedContentTransitionScope.isOverlayPopEnterTransition(): Boolean { - val initialRoute = initialState.destination.route - val targetRoute = targetState.destination.route - - val targetIsModuleItemSequence = targetRoute?.startsWith(MainNavigationRoute.ModuleItemSequence::class.java.name.replace("$", ".")).orDefault() - val isFromNotebook = initialRoute?.startsWith(NotebookRoute.Notebook.route).orDefault() - val isFromAddNote = initialRoute?.startsWith(NotebookRoute.AddNotebook::class.java.name.replace("$", ".")).orDefault() - val isFromEditNote = initialRoute?.startsWith(NotebookRoute.EditNotebook::class.java.name.replace("$", ".")).orDefault() - - return targetIsModuleItemSequence && (isFromNotebook || isFromAddNote || isFromEditNote) -} - -private fun AnimatedContentTransitionScope.isOverlayPopExitTransition(): Boolean { - val initialRoute = initialState.destination.route - val targetRoute = targetState.destination.route - - val isFromModuleItemSequence = initialRoute?.startsWith(MainNavigationRoute.ModuleItemSequence::class.java.name.replace("$", ".")).orDefault() - val targetIsNotebook = targetRoute?.startsWith(NotebookRoute.Notebook.route).orDefault() - val targetIsAddNote = targetRoute?.startsWith(NotebookRoute.AddNotebook::class.java.name.replace("$", ".")).orDefault() - val targetIsEditNote = targetRoute?.startsWith(NotebookRoute.EditNotebook::class.java.name.replace("$", ".")).orDefault() - - return isFromModuleItemSequence && !(targetIsNotebook || targetIsAddNote || targetIsEditNote) -} \ No newline at end of file diff --git a/libs/horizon/src/main/java/com/instructure/horizon/navigation/HorizonNavigationTransitionRules.kt b/libs/horizon/src/main/java/com/instructure/horizon/navigation/HorizonNavigationTransitionRules.kt new file mode 100644 index 0000000000..4e67353a42 --- /dev/null +++ b/libs/horizon/src/main/java/com/instructure/horizon/navigation/HorizonNavigationTransitionRules.kt @@ -0,0 +1,90 @@ +/* + * Copyright (C) 2025 - present Instructure, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, version 3 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ +package com.instructure.horizon.navigation + +import com.instructure.horizon.features.account.navigation.AccountRoute +import com.instructure.horizon.features.home.HomeNavigationRoute +import com.instructure.horizon.features.inbox.navigation.HorizonInboxRoute +import com.instructure.horizon.features.notebook.navigation.NotebookRoute +import com.instructure.horizon.horizonui.animation.NavigationTransitionAnimation + +data class NavigationTransitionAnimationRule( + val from: String? = null, + val to: String? = null, + val style: NavigationTransitionAnimation, +) + +val animationRules = listOf( + //Notebook + NavigationTransitionAnimationRule( + from = NotebookRoute.Notebook.route, + to = NotebookRoute.EditNotebook.serializableRoute, + style = NavigationTransitionAnimation.SCALE, + ), + NavigationTransitionAnimationRule( + from = NotebookRoute.Notebook.route, + to = NotebookRoute.AddNotebook.serializableRoute, + style = NavigationTransitionAnimation.SCALE + ), + NavigationTransitionAnimationRule( + from = MainNavigationRoute.ModuleItemSequence.serializableRoute, + to = NotebookRoute.Notebook.route, + style = NavigationTransitionAnimation.SCALE + ), + + //Account + NavigationTransitionAnimationRule( + from = HomeNavigationRoute.Account.route, + to = AccountRoute.Advanced.route, + style = NavigationTransitionAnimation.SLIDE + ), + NavigationTransitionAnimationRule( + from = HomeNavigationRoute.Account.route, + to = AccountRoute.BugReportWebView.route, + style = NavigationTransitionAnimation.SLIDE + ), + NavigationTransitionAnimationRule( + from = HomeNavigationRoute.Account.route, + to = AccountRoute.CalendarFeed.route, + style = NavigationTransitionAnimation.SLIDE + ), + NavigationTransitionAnimationRule( + from = HomeNavigationRoute.Account.route, + to = AccountRoute.Notifications.route, + style = NavigationTransitionAnimation.SLIDE + ), + NavigationTransitionAnimationRule( + from = HomeNavigationRoute.Account.route, + to = AccountRoute.Password.route, + style = NavigationTransitionAnimation.SLIDE + ), + NavigationTransitionAnimationRule( + from = HomeNavigationRoute.Account.route, + to = AccountRoute.Profile.route, + style = NavigationTransitionAnimation.SLIDE + ), + + //Inbox + NavigationTransitionAnimationRule( + from = null, + to = HorizonInboxRoute.InboxCompose.route, + style = NavigationTransitionAnimation.SCALE + ), +) + +private val Any.serializableRoute: String + get() = this::class.java.name.replace("$", ".").replace(".Companion", "") \ No newline at end of file From acf6e0ee799f25a8fe229661738df9209525e6e7 Mon Sep 17 00:00:00 2001 From: domonkosadam Date: Fri, 28 Nov 2025 08:55:19 +0100 Subject: [PATCH 33/33] Fix findings --- .../features/notebook/NotebookScreen.kt | 4 +-- .../horizonui/molecules/DropdownChip.kt | 30 ++++++++++--------- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt index ea8cecfccc..4e03a3aafa 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt @@ -334,10 +334,10 @@ private fun NoteContent( .fillMaxWidth() .horizonBorder( colorResource(note.type.color).copy(alpha = 0.1f), - 12.dp, + 6.dp, 1.dp, 1.dp, - 12.dp, + 6.dp, 16.dp ) .background( diff --git a/libs/horizon/src/main/java/com/instructure/horizon/horizonui/molecules/DropdownChip.kt b/libs/horizon/src/main/java/com/instructure/horizon/horizonui/molecules/DropdownChip.kt index 964e6a273b..4b75991219 100644 --- a/libs/horizon/src/main/java/com/instructure/horizon/horizonui/molecules/DropdownChip.kt +++ b/libs/horizon/src/main/java/com/instructure/horizon/horizonui/molecules/DropdownChip.kt @@ -160,21 +160,23 @@ fun DropdownChip( } } - InputDropDownPopup( - isMenuOpen = isMenuOpen, - options = items, - width = width, - verticalOffsetPx = heightInPx, - onMenuOpenChanged = { isMenuOpen = it }, - onOptionSelected = { item -> - if (selectedItem != item) { - onItemSelected(item) + if (enabled) { + InputDropDownPopup( + isMenuOpen = isMenuOpen, + options = items, + width = width, + verticalOffsetPx = heightInPx, + onMenuOpenChanged = { isMenuOpen = it }, + onOptionSelected = { item -> + if (selectedItem != item) { + onItemSelected(item) + } + }, + item = { item -> + DropdownChipItem(item, selectedItem) } - }, - item = { item -> - DropdownChipItem(item, selectedItem) - } - ) + ) + } } }