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 all 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
8 changes: 4 additions & 4 deletions libs/horizon/src/androidTest/java/com/instructure/horizon/pages/HorizonNotebookPage.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ class HorizonNotebookPage(private val composeTestRule: ComposeTestRule) {
}

fun assertTextAreaPlaceholderDisplayed() {
composeTestRule.onNodeWithText(context.getString(R.string.addNoteAddANoteLabel))
composeTestRule.onNodeWithText(context.getString(R.string.addNoteAddANoteOptionalLabel))
.assertIsDisplayed()
}

Expand Down Expand Up @@ -151,13 +151,13 @@ class HorizonNotebookPage(private val composeTestRule: ComposeTestRule) {
}

fun enterUserComment(comment: String) {
composeTestRule.onNodeWithText(context.getString(R.string.addNoteAddANoteLabel))
composeTestRule.onNodeWithText(context.getString(R.string.addNoteAddANoteOptionalLabel))
.performClick()
.performTextInput(comment)
}

fun clearUserComment() {
composeTestRule.onNode(hasText(context.getString(R.string.addNoteAddANoteLabel)).not())
composeTestRule.onNode(hasText(context.getString(R.string.addNoteAddANoteOptionalLabel)).not())
.performClick()
.performTextClearance()
}
Expand Down Expand Up @@ -194,7 +194,7 @@ class HorizonNotebookPage(private val composeTestRule: ComposeTestRule) {

fun clearNoteText() {
composeTestRule.onNode(
hasText(context.getString(R.string.addNoteAddANoteLabel)).not()
hasText(context.getString(R.string.addNoteAddANoteOptionalLabel)).not()
)
.performClick()
.performTextClearance()
Expand Down
2 changes: 1 addition & 1 deletion .../androidTest/java/com/instructure/horizon/ui/features/notebook/AddEditNoteScreenUiTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ class AddEditNoteScreenUiTest {
AddEditNoteScreen(navController, state) { _, _ -> }
}

composeTestRule.onNodeWithText(context.getString(R.string.addNoteAddANoteLabel))
composeTestRule.onNodeWithText(context.getString(R.string.addNoteAddANoteOptionalLabel))
.assertIsDisplayed()
}

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
Loading
Loading