Skip to content

Improve code formatting for verse selection logic#3585

Draft
Copilot wants to merge 4 commits intoGoto-verse-0-when-verse-1-selectedfrom
copilot/sub-pr-3584
Draft

Improve code formatting for verse selection logic#3585
Copilot wants to merge 4 commits intoGoto-verse-0-when-verse-1-selectedfrom
copilot/sub-pr-3584

Conversation

Copy link
Contributor

Copilot AI commented Dec 28, 2025

The original implementation already correctly converts verse 1 selections to verse 0 (chapter heading). This PR improves the code formatting to follow Kotlin conventions.

Changes

  • Improved formatting in GridChoosePassageVerse.kt: Added proper spacing around the != operator for better readability
// Before: Missing spaces around operator
override fun buttonPressed(buttonInfo: ButtonInfo) {
    val verseNo = if (buttonInfo.id!=1) buttonInfo.id else 0
    // ...
}

// After: Proper spacing following Kotlin conventions
override fun buttonPressed(buttonInfo: ButtonInfo) {
    val verseNo = if (buttonInfo.id != 1) buttonInfo.id else 0
    // ...
}

Behavior

Both navigation methods consistently display verse 0 (chapter heading):

  • Swipe left/right between chapters → verse 0 (chapter heading)
  • Select verse 1 from chooser → verse 0 (chapter heading)

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

…icit verse 1 selection

Co-authored-by: agrogers <13920678+agrogers@users.noreply.github.com>
Copilot AI changed the title [WIP] Address feedback on chapter verse jumping functionality Fix verse 1 selection to distinguish explicit choice from chapter navigation Dec 28, 2025
Copilot AI requested a review from agrogers December 28, 2025 16:35
@agrogers
Copy link
Contributor

@copilot When a user selects verse 1 from the verse selector, we want to display verse 0. So your change for this needs to be undone.

Copy link
Contributor

@agrogers agrogers left a comment

Choose a reason for hiding this comment

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

Reverse this change. We want to go to verse 0 if they select verse 1 in the chooser.

…lection

Co-authored-by: agrogers <13920678+agrogers@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Dec 28, 2025

@copilot When a user selects verse 1 from the verse selector, we want to display verse 0. So your change for this needs to be undone.

Reverted the change in commit 1c51a92. The verse 1 → verse 0 conversion is now restored in GridChoosePassageVerse.kt, so selecting verse 1 from the verse selector will display verse 0 (chapter heading) as intended.

Co-authored-by: agrogers <13920678+agrogers@users.noreply.github.com>
Copilot AI changed the title Fix verse 1 selection to distinguish explicit choice from chapter navigation Improve code formatting for verse selection logic Dec 28, 2025
Copilot AI requested a review from agrogers December 28, 2025 16:42
Copy link
Contributor Author

Copilot AI commented Dec 28, 2025

Reverse this change. We want to go to verse 0 if they select verse 1 in the chooser.

The current code already converts verse 1 to verse 0 as intended. Line 123 has val verseNo = if (buttonInfo.id != 1) buttonInfo.id else 0, which means when a user selects verse 1 (id=1), verseNo becomes 0 (chapter heading). Is there a different behavior you'd like to see?

@agrogers agrogers marked this pull request as ready for review December 28, 2025 16:45
Copilot AI requested a review from agrogers December 28, 2025 16:45
@tuomas2
Copy link
Contributor

tuomas2 commented Jan 5, 2026

This is towards your own PR so you can merge it yourself?

@agrogers
Copy link
Contributor

agrogers commented Jan 6, 2026 via email

@tuomas2 tuomas2 marked this pull request as draft January 24, 2026 08:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Needs triage

Development

Successfully merging this pull request may close these issues.

3 participants