Skip to content

[CLX-3438][Horizon] Notebook changes #3427

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Open
domonkosadam wants to merge 14 commits into
base: master
Choose a base branch
from
Open

[CLX-3438][Horizon] Notebook changes #3427

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
e66845d
Change icons
domonkosadam Dec 1, 2025
df0abb3
Scroll to selected note
domonkosadam Dec 2, 2025
48ae3f2
Refactor add note flow
domonkosadam Dec 2, 2025
215c891
Fix tests
domonkosadam Dec 2, 2025
142fab6
a11y improvements
domonkosadam Dec 3, 2025
f7a7205
a11y fix
domonkosadam Dec 4, 2025
f676ccd
Fix highlight scrolling
domonkosadam Dec 4, 2025
c1a142e
Fix webview scrolling
domonkosadam Dec 8, 2025
13366b1
Fix scrolling
domonkosadam Dec 8, 2025
088594e
Fix product findings
domonkosadam Dec 9, 2025
54ab8c3
Fix caching
domonkosadam Dec 9, 2025
e045726
Remove debug logs
domonkosadam Dec 9, 2025
3177b70
Fix tests
domonkosadam Dec 11, 2025
1c2e97d
Remove shadow from modal button
domonkosadam Dec 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion ...anvas-api-2/src/main/java/com/instructure/canvasapi2/DomainServicesGraphQLClientConfig.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ class RedwoodGraphQLClientConfig @Inject constructor(
adapter: RedwoodAdapter
): DomainServicesGraphQLClientConfig(
url = BuildConfig.REDWOOD_BASE_URL + "/graphql",
httpClient = adapter.buildOHttpClient()
httpClient = adapter.buildOHttpClient(),
fetchPolicy = HttpFetchPolicy.CacheFirst
)

class JourneyGraphQLClientConfig @Inject constructor(
Expand Down
2 changes: 1 addition & 1 deletion libs/horizon/src/main/java/com/instructure/horizon/features/dashboard/DashboardScreen.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ private fun HomeScreenTopBar(uiState: DashboardUiState, mainNavController: NavCo
)
Spacer(modifier = Modifier.weight(1f))
IconButton(
iconRes = R.drawable.menu_book_notebook,
iconRes = R.drawable.edit_note,
contentDescription = stringResource(R.string.a11y_dashboardNotebookButtonContentDescription),
onClick = {
mainNavController.navigate(MainNavigationRoute.Notebook.route)
Expand Down
2 changes: 1 addition & 1 deletion ...a/com/instructure/horizon/features/dashboard/widget/timespent/DashboardTimeSpentWidget.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ fun DashboardTimeSpentSection(
}
DashboardItemState.ERROR -> {
DashboardWidgetCardError(
stringResource(R.string.dashboardTimeSpentTitle),
stringResource(R.string.dashboardTimeLearningTitle),
R.drawable.schedule,
HorizonColors.PrimitivesHoney.honey12(),
false,
Expand Down
2 changes: 1 addition & 1 deletion ...ructure/horizon/features/dashboard/widget/timespent/card/DashboardTimeSpentCardContent.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ fun DashboardTimeSpentCardContent(
modifier: Modifier = Modifier,
) {
DashboardWidgetCard(
stringResource(R.string.dashboardTimeSpentTitle),
stringResource(R.string.dashboardTimeLearningTitle),
R.drawable.schedule,
HorizonColors.PrimitivesHoney.honey12(),
modifier,
Expand Down
21 changes: 17 additions & 4 deletions ...main/java/com/instructure/horizon/features/moduleitemsequence/ModuleItemSequenceScreen.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,9 @@ private fun ModuleItemSequenceContent(
moduleHeaderHeight = coordinates.size.height
val temp = nestedScrollConnection.appBarOffset
nestedScrollConnection =
CollapsingAppBarNestedScrollConnection(moduleHeaderHeight).apply { appBarOffset = temp }
CollapsingAppBarNestedScrollConnection(moduleHeaderHeight).apply {
appBarOffset = temp
}
}
}
) {
Expand All @@ -274,7 +276,10 @@ private fun ModuleItemSequenceContent(
containerColor = Color.Transparent,
modifier = Modifier
.conditional(uiState.loadingState.isLoading || uiState.loadingState.isError) {
background(color = HorizonColors.Surface.pageSecondary(), shape = HorizonCornerRadius.level5)
background(
color = HorizonColors.Surface.pageSecondary(),
shape = HorizonCornerRadius.level5
)
}
.padding(top = moduleHeaderHeight)
) {
Expand Down Expand Up @@ -305,6 +310,7 @@ private fun ModuleItemSequenceContent(
uiState.assignmentToolsOpened,
updateAiContext = uiState.updateAiAssistContext,
updateNotebookContext = uiState.updateObjectTypeAndId,
scrollToNoteId = uiState.scrollToNoteId
)
}
}
Expand Down Expand Up @@ -403,6 +409,7 @@ private fun ModuleItemContentScreen(
assignmentToolsOpened: () -> Unit,
updateAiContext: (AiAssistContextSource, String) -> Unit,
updateNotebookContext: (Pair<String, String>) -> Unit,
scrollToNoteId: String?,
modifier: Modifier = Modifier
) {
if (moduleItemUiState.isLoading) {
Expand Down Expand Up @@ -461,7 +468,8 @@ private fun ModuleItemContentScreen(
uiState = uiState,
scrollState = scrollState,
updateAiContext = { source, content -> updateAiContext(source, content) },
mainNavController = mainNavController
mainNavController = mainNavController,
scrollToNoteId = scrollToNoteId
)
}
composable(
Expand Down Expand Up @@ -546,6 +554,7 @@ private fun ModuleItemSequenceBottomBar(
) {
if (showPreviousButton) IconButton(
iconRes = R.drawable.chevron_left,
contentDescription = stringResource(R.string.a11y_previousModuleItem),
color = IconButtonColor.Inverse,
elevation = HorizonElevation.level4,
onClick = onPreviousClick,
Expand All @@ -559,20 +568,23 @@ private fun ModuleItemSequenceBottomBar(
) {
IconButton(
iconRes = R.drawable.ai,
contentDescription = stringResource(R.string.a11y_openAIAssistant),
enabled = aiAssistEnabled,
color = IconButtonColor.Ai,
elevation = HorizonElevation.level4,
onClick = onAiAssistClick,
)
if (showNotebookButton) IconButton(
iconRes = R.drawable.menu_book_notebook,
iconRes = R.drawable.edit_note,
contentDescription = stringResource(R.string.a11y_openNotebook),
enabled = notebookEnabled,
color = IconButtonColor.Inverse,
elevation = HorizonElevation.level4,
onClick = onNotebookClick,
)
if (showAssignmentToolsButton) IconButton(
iconRes = R.drawable.more_vert,
contentDescription = stringResource(R.string.a11y_openMoreOptions),
color = IconButtonColor.Inverse,
elevation = HorizonElevation.level4,
onClick = onAssignmentToolsClick,
Expand All @@ -587,6 +599,7 @@ private fun ModuleItemSequenceBottomBar(
}
if (showNextButton) IconButton(
iconRes = R.drawable.chevron_right,
contentDescription = stringResource(R.string.a11y_nextModuleItem),
color = IconButtonColor.Inverse,
elevation = HorizonElevation.level4,
onClick = onNextClick,
Expand Down
1 change: 1 addition & 0 deletions ...ain/java/com/instructure/horizon/features/moduleitemsequence/ModuleItemSequenceUiState.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ data class ModuleItemSequenceUiState(
val hasUnreadComments: Boolean = false,
val updateAiAssistContext: (AiAssistContextSource, String) -> Unit = { _, _ -> },
val shouldRefreshPreviousScreen: Boolean = false,
val scrollToNoteId: String? = null
)

data class ModuleItemUiState(
Expand Down
2 changes: 2 additions & 0 deletions ...n/java/com/instructure/horizon/features/moduleitemsequence/ModuleItemSequenceViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,15 @@ class ModuleItemSequenceViewModel @Inject constructor(
private val moduleItemId = savedStateHandle.toRoute<MainNavigationRoute.ModuleItemSequence>().moduleItemId
private val moduleItemAssetType = savedStateHandle.toRoute<MainNavigationRoute.ModuleItemSequence>().moduleItemAssetType
private val moduleItemAssetId = savedStateHandle.toRoute<MainNavigationRoute.ModuleItemSequence>().moduleItemAssetId
private val scrollToNoteId = savedStateHandle.toRoute<MainNavigationRoute.ModuleItemSequence>().scrollToNoteId

private var courseProgressChanged = false

private val _uiState =
MutableStateFlow(
ModuleItemSequenceUiState(
courseId = courseId,
scrollToNoteId = scrollToNoteId,
loadingState = LoadingState(onRefresh = ::refresh),
onPreviousClick = ::previousClicked,
onNextClick = ::nextClicked,
Expand Down
36 changes: 21 additions & 15 deletions .../instructure/horizon/features/moduleitemsequence/content/page/PageDetailsContentScreen.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ import androidx.compose.ui.draw.clip
import androidx.compose.ui.platform.LocalContext
import androidx.compose.ui.unit.dp
import androidx.navigation.NavHostController
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.horizon.features.aiassistant.common.model.AiAssistContextSource
import com.instructure.horizon.features.notebook.common.webview.ComposeNotesHighlightingCanvasWebView
import com.instructure.horizon.features.notebook.common.webview.NotesCallback
Expand All @@ -51,7 +54,8 @@ fun PageDetailsContentScreen(
scrollState: ScrollState,
updateAiContext: (AiAssistContextSource, String) -> Unit,
mainNavController: NavHostController,
modifier: Modifier = Modifier
scrollToNoteId: String?,
modifier: Modifier = Modifier,
) {
val activity = LocalContext.current.getActivityOrNull()
LaunchedEffect(uiState.urlToOpen) {
Expand Down Expand Up @@ -81,6 +85,7 @@ fun PageDetailsContentScreen(
ComposeNotesHighlightingCanvasWebView(
content = "<div id=\"parent-container\"><div>$it</div></div>",
notes = uiState.notes,
scrollToNoteId = scrollToNoteId,
applyOnWebView = {
activity?.let { addVideoClient(it) }
overrideHtmlFormatColors = HorizonColors.htmlFormatColors
Expand Down Expand Up @@ -112,20 +117,21 @@ fun PageDetailsContentScreen(
)
},
onNoteAdded = { selectedText, noteType, startContainer, startOffset, endContainer, endOffset, textSelectionStart, textSelectionEnd ->
mainNavController.navigate(
NotebookRoute.AddNotebook(
courseId = uiState.courseId.toString(),
objectType = "Page",
objectId = uiState.pageId.toString(),
highlightedTextStartOffset = startOffset,
highlightedTextEndOffset = endOffset,
highlightedTextStartContainer = startContainer,
highlightedTextEndContainer = endContainer,
highlightedText = selectedText,
textSelectionStart = textSelectionStart,
textSelectionEnd = textSelectionEnd,
noteType = noteType
)
uiState.addNote(
Copy link

@claude claude bot Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The onNoteAdded callback now calls uiState.addNote() instead of navigating directly. This is good architectural change, but ensure that:

  1. The addNote function properly handles all the navigation and state management
  2. Error handling is in place if the note creation fails
  3. The user receives feedback about the note creation status

Could you verify that uiState.addNote is implemented and handles these cases?

NoteHighlightedData(
selectedText = selectedText,
range = NoteHighlightedDataRange(
startOffset = startOffset,
endOffset = endOffset,
startContainer = startContainer,
endContainer = endContainer
),
textPosition = NoteHighlightedDataTextPosition(
start = textSelectionStart,
end = textSelectionEnd
)
),
noteType
)
}
)
Expand Down
14 changes: 12 additions & 2 deletions libs/horizon/src/main/java/com/instructure/horizon/features/notebook/NotebookScreen.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ 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.semantics.semantics
import androidx.compose.ui.text.style.TextOverflow
import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.unit.dp
Expand Down Expand Up @@ -216,6 +217,7 @@ fun NotebookScreen(
courseId = note.courseId,
moduleItemAssetType = note.objectType.value,
moduleItemAssetId = note.objectId,
scrollToNoteId = note.id
)
)
}
Expand Down Expand Up @@ -303,7 +305,15 @@ private fun FilterContent(
}

if (state.showNoteTypeFilter) {
NotebookTypeSelect(state.selectedFilter, state.onFilterSelected, false, true)
NotebookTypeSelect(
state.selectedFilter,
state.onFilterSelected,
false,
true,
Modifier.conditional(!state.showCourseFilter) { // TalkBack hack to fix focus handling
semantics(true) {}
}
)
}
}
}
Expand Down Expand Up @@ -333,7 +343,7 @@ private fun NoteContent(
modifier = Modifier
.fillMaxWidth()
.horizonBorder(
colorResource(note.type.color).copy(alpha = 0.1f),
colorResource(note.type.highlightColor),
6.dp,
1.dp,
1.dp,
Expand Down
47 changes: 41 additions & 6 deletions ...izon/src/main/java/com/instructure/horizon/features/notebook/addedit/AddEditNoteScreen.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package com.instructure.horizon.features.notebook.addedit

import androidx.activity.compose.BackHandler
import androidx.compose.foundation.focusable
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.PaddingValues
Expand All @@ -33,10 +34,19 @@ 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.saveable.rememberSaveable
import androidx.compose.runtime.setValue
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.focus.FocusRequester
import androidx.compose.ui.focus.focusRequester
import androidx.compose.ui.platform.LocalContext
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.semantics.isTraversalGroup
import androidx.compose.ui.semantics.semantics
import androidx.compose.ui.semantics.traversalIndex
import androidx.compose.ui.text.input.TextFieldValue
import androidx.compose.ui.text.style.TextOverflow
import androidx.compose.ui.tooling.preview.Preview
Expand Down Expand Up @@ -69,6 +79,7 @@ import com.instructure.horizon.horizonui.organisms.inputs.textarea.TextArea
import com.instructure.horizon.horizonui.organisms.inputs.textarea.TextAreaState
import com.instructure.pandautils.utils.ViewStyler
import com.instructure.pandautils.utils.getActivityOrNull
import kotlinx.coroutines.delay

@Composable
fun AddEditNoteScreen(
Expand Down Expand Up @@ -121,6 +132,15 @@ private fun AddEditNoteAppBar(
state: AddEditNoteUiState,
navigateBack: () -> Unit
) {
val requester = FocusRequester()
var requestFocus by rememberSaveable { mutableStateOf(true) }
LaunchedEffect(Unit) {
delay(50)
if(requestFocus) {
requester.requestFocus()
requestFocus = false
}
}
CenterAlignedTopAppBar(
colors = TopAppBarDefaults.topAppBarColors(
containerColor = HorizonColors.Surface.pageSecondary(),
Expand All @@ -131,7 +151,13 @@ private fun AddEditNoteAppBar(
Text(
state.title,
style = HorizonTypography.h4,
color = HorizonColors.Text.title()
color = HorizonColors.Text.title(),
modifier = Modifier
.semantics {
traversalIndex = -1f
}
.focusRequester(requester)
.focusable()
)
},
navigationIcon = {
Expand All @@ -158,7 +184,12 @@ private fun AddEditNoteAppBar(
enabled = state.hasContentChange && !state.isLoading
)
},
modifier = Modifier.padding(horizontal = 16.dp)
modifier = Modifier
.padding(horizontal = 16.dp)
.semantics {
isTraversalGroup = true
traversalIndex = -1f
}
)
}

Expand All @@ -174,7 +205,12 @@ private fun AddEditNoteContent(state: AddEditNoteUiState, padding: PaddingValues
) {
HorizonSpace(SpaceSize.SPACE_24)

NotebookTypeSelect(state.type, state.onTypeChanged, true, false)
NotebookTypeSelect(
state.type,
state.onTypeChanged,
true,
false,
)

HorizonSpace(SpaceSize.SPACE_24)

Expand All @@ -187,13 +223,12 @@ private fun AddEditNoteContent(state: AddEditNoteUiState, padding: PaddingValues

TextArea(
state = TextAreaState(
placeHolderText = stringResource(R.string.addNoteAddANoteLabel),
placeHolderText = stringResource(R.string.addNoteAddANoteOptionalLabel),
required = InputLabelRequired.Optional,
value = state.userComment,
onValueChange = state.onUserCommentChanged,
),
minLines = 5,
maxLines = 5
)

HorizonSpace(SpaceSize.SPACE_16)
Expand All @@ -205,7 +240,7 @@ private fun AddEditNoteContent(state: AddEditNoteUiState, padding: PaddingValues
if (state.lastModifiedDate != null) {
Text(
state.lastModifiedDate,
style = HorizonTypography.labelSmall,
style = HorizonTypography.labelMediumBold,
color = HorizonColors.Text.timestamp(),
modifier = Modifier.weight(1f),
maxLines = 1,
Expand Down
3 changes: 3 additions & 0 deletions ...n/src/main/java/com/instructure/horizon/features/notebook/addedit/add/AddNoteViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,13 @@ 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.addedit.AddEditViewModel
import com.instructure.horizon.features.notebook.common.composable.toNotebookLocalisedDateFormat
import com.instructure.horizon.features.notebook.common.model.NotebookType
import com.instructure.horizon.features.notebook.navigation.NotebookRoute
import dagger.hilt.android.lifecycle.HiltViewModel
import dagger.hilt.android.qualifiers.ApplicationContext
import kotlinx.coroutines.flow.update
import java.util.Date
import javax.inject.Inject

@HiltViewModel
Expand All @@ -57,6 +59,7 @@ class AddNoteViewModel @Inject constructor(
it.copy(
title = context.getString(R.string.createNoteTitle),
hasContentChange = true,
lastModifiedDate = Date().toNotebookLocalisedDateFormat(),
Copy link

@claude claude bot Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Creating a new Date() here means the lastModifiedDate is set when the ViewModel initializes, not when the note is actually saved. This could lead to inconsistent timestamps if the user takes time to write their note. Consider moving this to the actual save operation in the parent class or wherever the note creation happens.

highlightedData = NoteHighlightedData(
selectedText = highlightedText,
range = NoteHighlightedDataRange(
Expand Down
Loading
Loading