Skip to content

fix: spacing and indention of toc #4770

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
codiearyan wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: spacing and indention of toc #4770

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
138 changes: 106 additions & 32 deletions core/src/main/java/org/kiwix/kiwixmobile/core/main/reader/ReaderScreen.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,7 @@ import org.kiwix.kiwixmobile.core.ui.components.KiwixButton
import org.kiwix.kiwixmobile.core.ui.components.KiwixFloatingActionButton
import org.kiwix.kiwixmobile.core.ui.components.KiwixSnackbarHost
import org.kiwix.kiwixmobile.core.ui.components.KiwixWebViewWithAppBarScrolling
import org.kiwix.kiwixmobile.core.ui.components.ONE
import org.kiwix.kiwixmobile.core.ui.components.ProgressBarStyle
import org.kiwix.kiwixmobile.core.ui.components.TWELVE
import org.kiwix.kiwixmobile.core.ui.models.ActionMenuItem
import org.kiwix.kiwixmobile.core.ui.models.IconItem
import org.kiwix.kiwixmobile.core.ui.models.IconItem.Drawable
Expand All @@ -136,14 +134,25 @@ import org.kiwix.kiwixmobile.core.ui.theme.DenimBlue800
import org.kiwix.kiwixmobile.core.ui.theme.KiwixTheme
import org.kiwix.kiwixmobile.core.ui.theme.MineShaftGray700
import org.kiwix.kiwixmobile.core.ui.theme.White
import androidx.compose.material3.HorizontalDivider
import org.kiwix.kiwixmobile.core.utils.ComposeDimens.CLOSE_ALL_TAB_BUTTON_BOTTOM_PADDING
import org.kiwix.kiwixmobile.core.utils.ComposeDimens.CLOSE_TAB_ICON_ANIMATION_TIMEOUT
import org.kiwix.kiwixmobile.core.utils.ComposeDimens.CLOSE_TAB_ICON_SIZE
import org.kiwix.kiwixmobile.core.utils.ComposeDimens.EIGHT_DP
import org.kiwix.kiwixmobile.core.utils.ComposeDimens.FIVE_DP
import org.kiwix.kiwixmobile.core.utils.ComposeDimens.FOUR_DP
import org.kiwix.kiwixmobile.core.utils.ComposeDimens.KIWIX_TOOLBAR_HEIGHT
import org.kiwix.kiwixmobile.core.utils.ComposeDimens.FIFTEEN_SP
import org.kiwix.kiwixmobile.core.utils.ComposeDimens.LARGE_BODY_TEXT_SIZE
import org.kiwix.kiwixmobile.core.utils.ComposeDimens.MEDIUM_BODY_TEXT_SIZE
import org.kiwix.kiwixmobile.core.utils.ComposeDimens.THIRTEEN_SP
import org.kiwix.kiwixmobile.core.utils.ComposeDimens.TOC_HEADING_LEVEL_2
import org.kiwix.kiwixmobile.core.utils.ComposeDimens.TOC_HEADING_LEVEL_3
import org.kiwix.kiwixmobile.core.utils.ComposeDimens.TOC_HEADING_LEVEL_4
import org.kiwix.kiwixmobile.core.utils.ComposeDimens.TOC_HEADER_VERTICAL_PADDING
import org.kiwix.kiwixmobile.core.utils.ComposeDimens.TOC_INDENT_PER_LEVEL
import org.kiwix.kiwixmobile.core.utils.ComposeDimens.TOC_ITEM_CORNER_RADIUS
import org.kiwix.kiwixmobile.core.utils.ComposeDimens.TOC_ITEM_VERTICAL_PADDING
import org.kiwix.kiwixmobile.core.utils.ComposeDimens.NAVIGATION_DRAWER_WIDTH
import org.kiwix.kiwixmobile.core.utils.ComposeDimens.ONE_DP
import org.kiwix.kiwixmobile.core.utils.ComposeDimens.READER_BOTTOM_APP_BAR_BUTTON_ICON_SIZE
Expand All @@ -154,7 +163,6 @@ import org.kiwix.kiwixmobile.core.utils.ComposeDimens.SIXTEEN_DP
import org.kiwix.kiwixmobile.core.utils.ComposeDimens.TEN_DP
import org.kiwix.kiwixmobile.core.utils.ComposeDimens.THREE_DP
import org.kiwix.kiwixmobile.core.utils.ComposeDimens.TTS_BUTTONS_CONTROL_ALPHA
import org.kiwix.kiwixmobile.core.utils.ComposeDimens.TWELVE_DP
import org.kiwix.kiwixmobile.core.utils.ComposeDimens.TWO_DP
import org.kiwix.kiwixmobile.core.utils.HUNDERED
import org.kiwix.kiwixmobile.core.utils.StyleUtils.fromHtml
Expand Down Expand Up @@ -261,7 +269,7 @@ fun ReaderScreen(
title = state.tableOfContentTitle,
sections = documentSections.orEmpty(),
state.selectedWebView,
showTableOfContentDrawer
showTableOfContentDrawer,
)
}
}
Expand Down Expand Up @@ -345,53 +353,63 @@ private fun ReaderContentLayout(
}
}

@Suppress("LongMethod")
@Suppress("LongMethod", "LongParameterList")
@Composable
fun TableDrawerSheet(
title: String,
sections: List<DocumentSection>,
selectedWebView: KiwixWebView?,
showTableOfContentDrawer: MutableState<Boolean>
showTableOfContentDrawer: MutableState<Boolean>,
) {
val drawerBackgroundColor = if (isSystemInDarkTheme()) {
MineShaftGray700
} else {
White
}
val lazyListState = rememberLazyListState()
ModalDrawerSheet(
modifier = Modifier.width(NAVIGATION_DRAWER_WIDTH),
drawerShape = RectangleShape,
drawerContainerColor = drawerBackgroundColor
) {
LazyColumn(modifier = Modifier.fillMaxHeight()) {
LazyColumn(
state = lazyListState,
modifier = Modifier.fillMaxHeight()
) {
item {
Text(
text = title.ifEmpty { stringResource(id = R.string.no_section_info) },
style = MaterialTheme.typography.titleMedium,
modifier = Modifier
.fillMaxWidth()
.minimumInteractiveComponentSize()
.clickable {
onTableOfContentHeaderClick(
selectedWebView,
showTableOfContentDrawer
)
}
.padding(horizontal = SIXTEEN_DP, vertical = TWELVE_DP)
.semantics { contentDescription = "${title}${title.hashCode()}" }
)
Column {
Text(
text = title.ifEmpty { stringResource(id = R.string.no_section_info) },
style = MaterialTheme.typography.titleLarge.copy(fontWeight = FontWeight.Bold),
modifier = Modifier
.fillMaxWidth()
.minimumInteractiveComponentSize()
.clickable {
onTableOfContentHeaderClick(
selectedWebView,
showTableOfContentDrawer
)
}
.padding(horizontal = SIXTEEN_DP, vertical = TOC_HEADER_VERTICAL_PADDING)
.semantics { contentDescription = "${title}${title.hashCode()}" }
)
HorizontalDivider(
thickness = ONE_DP,
color = MaterialTheme.colorScheme.outlineVariant
)
}
}
itemsIndexed(sections) { index, section ->
val paddingStart = (section.level - ONE) * TWELVE
Text(
text = section.title,
style = MaterialTheme.typography.bodyMedium.copy(
fontWeight = FontWeight.Light,
fontSize = LARGE_BODY_TEXT_SIZE
),
val sectionStyle = tocSectionStyle(section.level)

Box(
modifier = Modifier
.fillMaxWidth()
.minimumInteractiveComponentSize()
.padding(horizontal = FOUR_DP)
.clip(RoundedCornerShape(TOC_ITEM_CORNER_RADIUS))
.then(
Modifier
)
Comment on lines +410 to +412

Copilot AI Apr 18, 2026

Copy link

Choose a reason for hiding this comment

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

The .then(Modifier) call is a no-op and adds noise to the modifier chain. It can be removed (or replaced with a real conditional modifier if something was intended here).

Suggested change
.then(
Modifier
)

Copilot uses AI. Check for mistakes.
Comment on lines +410 to +412

Copilot AI Apr 18, 2026

Copy link

Choose a reason for hiding this comment

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

TOC rows no longer apply minimumInteractiveComponentSize(). With the reduced vertical padding, items can fall below the 48dp minimum touch target, which hurts accessibility/usability. Consider re-adding minimumInteractiveComponentSize() (or otherwise enforcing a min height) on the clickable row container.

Suggested change
.then(
Modifier
)
.minimumInteractiveComponentSize()

Copilot uses AI. Check for mistakes.
.clickable {
onTableOfContentSectionClick(
selectedWebView,
Expand All @@ -400,14 +418,70 @@ fun TableDrawerSheet(
showTableOfContentDrawer
)
}
.padding(start = paddingStart.dp, top = EIGHT_DP, bottom = EIGHT_DP, end = SIXTEEN_DP)
.padding(
start = sectionStyle.startPadding,
top = TOC_ITEM_VERTICAL_PADDING,
bottom = TOC_ITEM_VERTICAL_PADDING,
end = SIXTEEN_DP
)
.semantics { contentDescription = "${section.title}$index" }
)
) {
Text(
text = section.title,
fontSize = sectionStyle.fontSize,
fontWeight = sectionStyle.fontWeight,
color = sectionStyle.color,
maxLines = 2,
overflow = TextOverflow.Ellipsis
)
}
}
}
}
}

private data class TocSectionStyle(
val fontSize: androidx.compose.ui.unit.TextUnit,
val fontWeight: FontWeight,
val color: Color,
val startPadding: Dp
)

@Composable
private fun tocSectionStyle(level: Int): TocSectionStyle {
val onSurface = MaterialTheme.colorScheme.onSurface
val onTertiary = MaterialTheme.colorScheme.onTertiary
return when (level) {
TOC_HEADING_LEVEL_2 -> TocSectionStyle(
fontSize = LARGE_BODY_TEXT_SIZE,
fontWeight = FontWeight.SemiBold,
color = onSurface,
startPadding = SIXTEEN_DP
)

TOC_HEADING_LEVEL_3 -> TocSectionStyle(
fontSize = FIFTEEN_SP,
fontWeight = FontWeight.Medium,
color = onSurface,
startPadding = SIXTEEN_DP + TOC_INDENT_PER_LEVEL
)

TOC_HEADING_LEVEL_4 -> TocSectionStyle(
fontSize = MEDIUM_BODY_TEXT_SIZE,
fontWeight = FontWeight.Normal,
color = onTertiary,
startPadding = SIXTEEN_DP + TOC_INDENT_PER_LEVEL * 2
)

else -> TocSectionStyle(
fontSize = THIRTEEN_SP,
fontWeight = FontWeight.Normal,
color = onTertiary,
startPadding = SIXTEEN_DP + TOC_INDENT_PER_LEVEL * 3
)
}
}

private fun onTableOfContentHeaderClick(
selectedWebView: KiwixWebView?,
showTableOfContentDrawer: MutableState<Boolean>
Expand Down
12 changes: 9 additions & 3 deletions core/src/main/java/org/kiwix/kiwixmobile/core/utils/ComposeDimens.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ object ComposeDimens {
val CRASH_IMAGE_SIZE = 70.dp

// KiwixAppBar(Toolbar) dimens
val ACTION_MENU_TEXTVIEW_BUTTON_PADDING = 13.dp
val KIWIX_TOOLBAR_HEIGHT = 56.dp
val KIWIX_TOOLBAR_SHADOW_ELEVATION = 4.dp

Expand All @@ -57,14 +56,12 @@ object ComposeDimens {
val THREE_DP = 3.dp
val TWO_DP = 2.dp
val ONE_DP = 1.dp
val SEVENTY_DP = 70.dp
val FIFTY_DP = 50.dp
val SIXTY_FOUR_DP = 64.dp
val ONE_HUNDRED_FIFTY = 150.dp
val TWENTY_TWO_DP = 22.dp

// Font Sizes
val TWENTY_FOUR_SP = 24.sp
val FOURTEEN_SP = 14.sp
val EIGHTEEN_SP = 18.sp

Expand Down Expand Up @@ -195,6 +192,15 @@ object ComposeDimens {
const val READER_BOTTOM_APP_BAR_DISABLE_BUTTON_ALPHA = 0.38f
val SEARCH_PLACEHOLDER_TEXT_SIZE = 12.sp
val DONATION_LAYOUT_MAXIMUM_WIDTH = 400.dp
val TOC_HEADER_VERTICAL_PADDING = 16.dp
val TOC_ITEM_VERTICAL_PADDING = 6.dp
val TOC_INDENT_PER_LEVEL = 12.dp
val TOC_ITEM_CORNER_RADIUS = 8.dp
val FIFTEEN_SP = 15.sp
val THIRTEEN_SP = 13.sp
const val TOC_HEADING_LEVEL_2 = 2
const val TOC_HEADING_LEVEL_3 = 3
const val TOC_HEADING_LEVEL_4 = 4

// MainActivity dimens
val NAVIGATION_DRAWER_WIDTH = 280.dp
Expand Down
Loading