Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ import com.bumptech.glide.integration.compose.ExperimentalGlideComposeApi
import com.bumptech.glide.integration.compose.GlideImage
import com.instructure.canvasapi2.utils.ContextKeeper
import com.instructure.horizon.R
import com.instructure.horizon.features.dashboard.widget.DashboardWidgetPageState
import com.instructure.horizon.features.dashboard.widget.announcement.DashboardAnnouncementBannerWidget
import com.instructure.horizon.features.dashboard.widget.course.DashboardCourseSection
import com.instructure.horizon.features.dashboard.widget.myprogress.DashboardMyProgressWidget
Expand Down Expand Up @@ -191,7 +192,8 @@ fun DashboardScreen(uiState: DashboardUiState, mainNavController: NavHostControl
refreshStateFlow
)
HorizonSpace(SpaceSize.SPACE_16)
val pagerState = rememberPagerState{ 3 }
val pageCount = 3
val pagerState = rememberPagerState{ pageCount }
AnimatedHorizontalPager(
pagerState,
sizeAnimationRange = 0f,
Expand All @@ -204,13 +206,15 @@ fun DashboardScreen(uiState: DashboardUiState, mainNavController: NavHostControl
DashboardMyProgressWidget(
shouldRefresh,
refreshStateFlow,
DashboardWidgetPageState(index + 1, pageCount),
modifier.padding(bottom = 16.dp)
)
}
1 -> {
DashboardTimeSpentWidget(
shouldRefresh,
refreshStateFlow,
DashboardWidgetPageState(index + 1, pageCount),
modifier.padding(bottom = 16.dp)
)
}
Expand All @@ -219,12 +223,10 @@ fun DashboardScreen(uiState: DashboardUiState, mainNavController: NavHostControl
homeNavController,
shouldRefresh,
refreshStateFlow,
DashboardWidgetPageState(index + 1, pageCount),
modifier.padding(bottom = 16.dp)
)
}
else -> {

}
}
Copy link

Choose a reason for hiding this comment

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

The else branch was removed from the when expression, but this makes the when non-exhaustive. If the pager ever has more than 3 pages or an invalid index, this could lead to silent failures. Consider either keeping the empty else branch for safety, or adding a check/comment explaining why indices 0-2 are guaranteed.

}
DashboardSkillHighlightsWidget(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ package com.instructure.horizon.features.dashboard.widget

import androidx.compose.foundation.layout.Column
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.pager.rememberPagerState
Expand All @@ -32,21 +30,18 @@ import androidx.compose.ui.res.stringResource
import androidx.compose.ui.semantics.Role
import androidx.compose.ui.semantics.role
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
import androidx.navigation.NavHostController
import androidx.navigation.compose.rememberNavController
import com.instructure.canvasapi2.utils.ContextKeeper
import com.instructure.horizon.R
import com.instructure.horizon.features.dashboard.DashboardCard
import com.instructure.horizon.horizonui.animation.shimmerEffect
import com.instructure.horizon.horizonui.foundation.HorizonColors
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.StatusChip
import com.instructure.horizon.horizonui.molecules.StatusChipColor
import com.instructure.horizon.horizonui.molecules.StatusChipState
import com.instructure.horizon.horizonui.organisms.AnimatedHorizontalPager
import com.instructure.pandautils.utils.localisedFormat
import java.util.Date
Expand All @@ -71,13 +66,22 @@ fun DashboardPaginatedWidgetCard(
pageSpacing = 12.dp,
verticalAlignment = Alignment.CenterVertically,
) { index, modifier ->
DashboardCard(
val item = state.items[index]
DashboardWidgetCard(
title = item.headerState.label,
iconRes = item.headerState.iconRes,
useMinWidth = false,
widgetColor = item.headerState.color,
pageState = DashboardWidgetPageState(
currentPageNumber = pagerState.currentPage + 1,
pageCount = state.items.size
),
modifier = modifier.padding(bottom = 16.dp),
onClick = if (state.isLoading) {
null
} else {
{
state.items[index].route?.let { route ->
item.route?.let { route ->
when (route) {
is DashboardPaginatedWidgetCardButtonRoute.HomeRoute -> {
homeNavController.navigate(route.route)
Expand All @@ -96,19 +100,14 @@ fun DashboardPaginatedWidgetCard(
modifier = Modifier
.fillMaxWidth()
) {
HorizonSpace(SpaceSize.SPACE_24)

DashboardPaginatedWidgetCardItem(
item = state.items[index],
item = item,
isLoading = state.isLoading,
modifier = Modifier
Copy link

Choose a reason for hiding this comment

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

The hardcoded padding removal here (from 24.dp horizontal padding) changes the visual alignment of the content. Please verify this was intentional and consistent with the design. The DashboardWidgetCard wrapper should be handling padding consistently.

.padding(horizontal = 24.dp)
.semantics(mergeDescendants = true) {
role = Role.Button
}
)

HorizonSpace(SpaceSize.SPACE_24)
}
}
}
Expand All @@ -124,44 +123,14 @@ private fun DashboardPaginatedWidgetCardItem(
Column(
modifier = modifier.fillMaxWidth()
) {
if (item.chipState != null || item.pageState != null) {
Row(
verticalAlignment = Alignment.CenterVertically,
modifier = Modifier.fillMaxWidth()
) {
item.chipState?.let { chipState ->
StatusChip(
state = StatusChipState(
label = chipState.label,
color = chipState.color,
fill = true
),
modifier = Modifier.shimmerEffect(
isLoading,
backgroundColor = chipState.color.fillColor.copy(0.8f),
shimmerColor = chipState.color.fillColor.copy(0.5f)
)
)
}

Spacer(Modifier.weight(1f))

item.pageState?.let {
Text(
it,
style = HorizonTypography.p2,
color = HorizonColors.Text.dataPoint(),
)
}
}
HorizonSpace(SpaceSize.SPACE_16)
}
item.source?.let { source ->
Text(
text = stringResource(
R.string.dashboardAnnouncementBannerFrom,
source
Copy link

Choose a reason for hiding this comment

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

Good improvement! Adding maxLines and overflow prevents text overflow issues. However, consider if 1 line is sufficient for course names which can be long. Might want to test with real data to ensure it doesn't truncate too aggressively.

),
maxLines = 1,
overflow = TextOverflow.Ellipsis,
style = HorizonTypography.p2,
color = HorizonColors.Text.dataPoint(),
modifier = Modifier.shimmerEffect(isLoading)
Expand All @@ -182,13 +151,13 @@ private fun DashboardPaginatedWidgetCardItem(
Text(
text = title,
style = HorizonTypography.p1,
maxLines = 3,
overflow = TextOverflow.Ellipsis,
color = HorizonColors.Text.body(),
modifier = Modifier
.fillMaxWidth()
.shimmerEffect(isLoading)
)

HorizonSpace(SpaceSize.SPACE_16)
}
}
}
Expand All @@ -201,29 +170,32 @@ private fun DashboardPaginatedWidgetCardAnnouncementContentPreview() {
state = DashboardPaginatedWidgetCardState(
items = listOf(
DashboardPaginatedWidgetCardItemState(
chipState = DashboardPaginatedWidgetCardChipState(
headerState = DashboardPaginatedWidgetCardHeaderState(
label = "Announcement",
color = StatusChipColor.Sky
color = HorizonColors.Surface.institution().copy(alpha = 0.1f),
iconRes = R.drawable.ic_announcement
),
title = "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Announcement title shown here.",
source = "Institution or Course Name Here",
date = Date(),
route = DashboardPaginatedWidgetCardButtonRoute.MainRoute("")
),
DashboardPaginatedWidgetCardItemState(
chipState = DashboardPaginatedWidgetCardChipState(
headerState = DashboardPaginatedWidgetCardHeaderState(
label = "Announcement",
color = StatusChipColor.Sky
color = HorizonColors.Surface.institution().copy(alpha = 0.1f),
iconRes = R.drawable.ic_announcement
),
title = "Second announcement with different content to show pagination.",
source = "Another Course Name",
date = Date(),
route = DashboardPaginatedWidgetCardButtonRoute.MainRoute("")
),
DashboardPaginatedWidgetCardItemState(
chipState = DashboardPaginatedWidgetCardChipState(
headerState = DashboardPaginatedWidgetCardHeaderState(
label = "Announcement",
color = StatusChipColor.Sky
color = HorizonColors.Surface.institution().copy(alpha = 0.1f),
iconRes = R.drawable.ic_announcement
),
title = "Third global announcement without a source.",
date = Date(),
Expand All @@ -241,21 +213,7 @@ private fun DashboardPaginatedWidgetCardAnnouncementContentPreview() {
private fun DashboardPaginatedWidgetCardAnnouncementLoadingPreview() {
ContextKeeper.appContext = LocalContext.current
DashboardPaginatedWidgetCard(
state = DashboardPaginatedWidgetCardState(
items = listOf(
DashboardPaginatedWidgetCardItemState(
chipState = DashboardPaginatedWidgetCardChipState(
label = "Announcement",
color = StatusChipColor.Sky
),
title = "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Announcement title shown here.",
source = "Institution or Course Name Here",
date = Date(),
route = DashboardPaginatedWidgetCardButtonRoute.MainRoute("")
),
),
isLoading = true
),
state = DashboardPaginatedWidgetCardState.Loading,
rememberNavController(),
rememberNavController(),
)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,25 +1,48 @@
package com.instructure.horizon.features.dashboard.widget

import com.instructure.horizon.horizonui.molecules.StatusChipColor
import androidx.annotation.DrawableRes
import androidx.compose.ui.graphics.Color
import com.instructure.horizon.R
import com.instructure.horizon.horizonui.foundation.HorizonColors
import java.util.Date

data class DashboardPaginatedWidgetCardState(
val items: List<DashboardPaginatedWidgetCardItemState> = emptyList(),
val isLoading: Boolean = false,
)
) {
companion object {
val Loading: DashboardPaginatedWidgetCardState
get() = DashboardPaginatedWidgetCardState(
items = listOf(
DashboardPaginatedWidgetCardItemState(
headerState = DashboardPaginatedWidgetCardHeaderState(
label = "Announcement",
color = HorizonColors.Surface.institution().copy(alpha = 0.1f),
iconRes = R.drawable.ic_announcement
),
title = "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Announcement title shown here.",
source = "Institution or Course Name Here",
date = Date(),
route = DashboardPaginatedWidgetCardButtonRoute.MainRoute("")
),
),
isLoading = true
)
}
}

data class DashboardPaginatedWidgetCardItemState(
val chipState: DashboardPaginatedWidgetCardChipState? = null,
val pageState: String? = null,
val headerState: DashboardPaginatedWidgetCardHeaderState,
val source: String? = null,
val date: Date? = null,
val title: String? = null,
val route: DashboardPaginatedWidgetCardButtonRoute? = null
)

data class DashboardPaginatedWidgetCardChipState(
data class DashboardPaginatedWidgetCardHeaderState(
val label: String,
val color: StatusChipColor,
val color: Color,
@DrawableRes val iconRes: Int,
)

sealed class DashboardPaginatedWidgetCardButtonRoute {
Expand Down
Loading
Loading