Skip to content

fix: spacing and indention of toc#4770

Open
codiearyan wants to merge 1 commit into
kiwix:mainfrom
codiearyan:feat-ui/improve-toc-4749
Open

fix: spacing and indention of toc#4770
codiearyan wants to merge 1 commit into
kiwix:mainfrom
codiearyan:feat-ui/improve-toc-4749

Conversation

@codiearyan

@codiearyan codiearyan commented Mar 12, 2026

Copy link
Copy Markdown
Contributor

Fixes #4749

  • Improving spacing and indentation of the TOC
  • Having Font Styles based on the TOC section level

Screenshots
image

@atharvyadav22

atharvyadav22 commented Mar 21, 2026

Copy link
Copy Markdown
Contributor

@codiearyan @MohitMaliFtechiz

CI is Failing for -

> Task :app:compileDebugUnitTestKotlin
e: file:///home/runner/work/kiwix-android/kiwix-android/app/src/test/java/org/kiwix/kiwixmobile/zimManager/ZimManageViewModelTest.kt:152:3 This annotation is not repeatable.
e: file:///home/runner/work/kiwix-android/kiwix-android/app/src/test/java/org/kiwix/kiwixmobile/zimManager/ZimManageViewModelTest.kt:419:3 This annotation is not repeatable.

In ZimManageViewModelTest

Line no 168

@Suppress("UnspecifiedRegisterReceiverFlag")
  every { application.registerReceiver(any(), any()) } returns mockk()

Moving this @Suppress("UnspecifiedRegisterReceiverFlag") to

  @Suppress("UnspecifiedRegisterReceiverFlag")
  @BeforeEach
  fun init() {

Line no 252

@Suppress("UnspecifiedRegisterReceiverFlag")
        verify {
          application.registerReceiver(connectivityBroadcastReceiver, any())
        }

Moving this @Suppress("UnspecifiedRegisterReceiverFlag") to

  @Suppress("UnspecifiedRegisterReceiverFlag")
  @Nested
  inner class Context {

Should work. won't be raising PR for such small things.
Thanks.

@codiearyan

codiearyan commented Mar 22, 2026

Copy link
Copy Markdown
Contributor Author

@codiearyan @MohitMaliFtechiz

CI is Failing for -

> Task :app:compileDebugUnitTestKotlin
e: file:///home/runner/work/kiwix-android/kiwix-android/app/src/test/java/org/kiwix/kiwixmobile/zimManager/ZimManageViewModelTest.kt:152:3 This annotation is not repeatable.
e: file:///home/runner/work/kiwix-android/kiwix-android/app/src/test/java/org/kiwix/kiwixmobile/zimManager/ZimManageViewModelTest.kt:419:3 This annotation is not repeatable.

In ZimManageViewModelTest

Line no 168

@Suppress("UnspecifiedRegisterReceiverFlag")
  every { application.registerReceiver(any(), any()) } returns mockk()

Moving this @Suppress("UnspecifiedRegisterReceiverFlag") to

  @Suppress("UnspecifiedRegisterReceiverFlag")
  @BeforeEach
  fun init() {

Line no 252

@Suppress("UnspecifiedRegisterReceiverFlag")
        verify {
          application.registerReceiver(connectivityBroadcastReceiver, any())
        }

Moving this @Suppress("UnspecifiedRegisterReceiverFlag") to

  @Suppress("UnspecifiedRegisterReceiverFlag")
  @Nested
  inner class Context {

Should work. won't be raising PR for such small things. Thanks.

ok thanks for this @atharvyadav22 , should i go ahead and fix this, and also it looks like there is a merge conflict too, so will fix that too

@kelson42

kelson42 commented Apr 14, 2026

Copy link
Copy Markdown
Collaborator

What is the status of this PR?

Copilot AI reviewed Apr 18, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Improves the Reader Table of Contents (TOC) drawer layout and typography to better reflect section hierarchy and spacing (Fixes #4749).

Changes:

  • Adds TOC-specific spacing/typography constants in ComposeDimens.
  • Updates TableDrawerSheet to introduce a styled TOC header divider, hierarchical indentation, and per-level font styling for TOC entries.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
core/src/main/java/org/kiwix/kiwixmobile/core/utils/ComposeDimens.kt Adds new TOC padding/indent/font-size constants and heading-level constants.
core/src/main/java/org/kiwix/kiwixmobile/core/main/reader/ReaderScreen.kt Refactors TOC drawer UI: new header layout, divider, per-level styling, and item indentation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread core/src/main/java/org/kiwix/kiwixmobile/core/main/reader/ReaderScreen.kt
Comment on lines +410 to +412
.then(
Modifier
)

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 thread core/src/main/java/org/kiwix/kiwixmobile/core/main/reader/ReaderScreen.kt
Comment on lines +410 to +412
.then(
Modifier
)

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.
@atharvyadav22

atharvyadav22 commented Apr 18, 2026

Copy link
Copy Markdown
Contributor

@MohitMaliFtechiz
if @codiearyan is busy with something can i take this one as we have similar PR of mine we can club it

@MohitMaliFtechiz

MohitMaliFtechiz commented Apr 21, 2026

Copy link
Copy Markdown
Collaborator

@atharvyadav22 Thanks. What do you think about the current UI(updated on the PR description)? Do you have any other reference for the UI?

@atharvyadav22

atharvyadav22 commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

@MohitMaliFtechiz @kelson42
What about this one? I'll add Material 3 Typography for Texts.
Screenshot_2026-04-24-13-16-12-97_40deb401b9ffe8e1df2f1cc5ba480b12_Edited

@harshsomankar123-tech

harshsomankar123-tech commented May 10, 2026

Copy link
Copy Markdown
Collaborator

@atharvyadav22 Thanks for the proposal! I really like the improved layout and hierarchy. My only concern is typography consistency.
Since the app still follows Material 2, introducing M3 typography only for the TOC may feel visually inconsistent with the rest of the UI.
What do you think?

@atharvyadav22

atharvyadav22 commented May 11, 2026

Copy link
Copy Markdown
Contributor

@harshsomankar123-tech
I'll check the UI and share you the design about how it looks shortly maybe we will be clear and come upto an decision.
Thanks.

@harshsomankar123-tech

harshsomankar123-tech commented May 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Hii @atharvyadav22
Is there any solution like this #4749 (comment) which can make our TOC look more clearer? The comment approach feels it may create some UX-related problems #4749 (comment).
WDYT??

@atharvyadav22

atharvyadav22 commented May 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Hii @atharvyadav22 Is there any solution like this #4749 (comment) which can make our TOC look more clearer? The comment approach feels it may create some UX-related problems #4749 (comment). WDYT??

We will hold this as of now when mentors are free we will have a look at this as it's just UX improvement.
If any of the team suggested we will fix it ASAP

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@MohitMaliFtechiz MohitMaliFtechiz Awaiting requested review from MohitMaliFtechiz

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Improve look & feel of the TOC

6 participants

@codiearyan @atharvyadav22 @kelson42 @MohitMaliFtechiz @harshsomankar123-tech