Install button labels picked release for not-yet-installed apps#566
Merged
Conversation
Reproducer: open details for an app you don't have installed, open the release picker, choose a non-latest release. The button stayed "Install latest" — confusing because tapping it would actually install the picked older release. The label switch only checked the installed-vs-selected branch (`isInstalled && normInstalled != null && ...`). When the app wasn't installed, that branch was skipped and the cascade fell straight through to "Install latest" regardless of which release the picker had selected. Add an intermediate branch: when the app isn't installed but the selection differs from `allReleases.first()` (GitHub returns the feed newest-first by `published_at`), render "Install version X". The existing "Install latest" stays as the fall-through for the genuine latest selection or when the comparison can't be done. Refs #538.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughSmartInstallButton normalizes the selected release tag for display, adds a button-text branch that shows "Install version X" when an uninstalled non-latest release is selected, and forces the button label to a single line with ellipsis. ChangesVersion-Specific Install Label
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/SmartInstallButton.kt`:
- Around line 260-263: The branch that checks normSelected vs latestTag can
wrongly fire for installed apps when the installed version is missing; update
the conditional that currently reads the normSelected &&
state.allReleases.firstOrNull()?.tagName... to also require !isInstalled so the
branch only runs for non-installed flows—locate the check around normSelected,
VersionMath.isExactSameVersion(latestTag, normSelected) and add the !isInstalled
guard to the combined condition.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1a4e0f23-6500-4396-bcdd-09c47a7409a1
📒 Files selected for processing (1)
feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/SmartInstallButton.kt
…th-style tags. Some maintainers tag releases with path-style strings such as `com.akylas.documentscanner/android/github/1.21.0/152`. Plugging that into the "Install version X" template wrapped the CTA across two lines and rendered visibly broken. Two stacked safety nets: - Derive `displaySelected` via `VersionMath.normalizeVersion`. For the OSS-DocumentScanner tag above it returns `1.21.0`. For conventional tags (`v1.2.3`, `1.2.3-rc1`, `release-1.2.0`) the output already matches what we'd render today, so the change is a no-op for the common case. The raw tag still flows through the install pipeline — only the display label uses the trimmed form. - Ellipsise the button Text (`maxLines = 1, overflow = Ellipsis`). When the trim heuristic doesn't help (commit-hash tags, weird schemes) the button stays single-line instead of breaking the row. Refs #538.
|
that ui glitch usthat ui glitch was this. |
Member
Author
|
@HikaruIchijyo Yeah that one has a long release tag name, thats why it broke, but now its fixed |
will see thanks for this |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Repro
Cause
SmartInstallButtononly switched to the "Install version X" label inside the isInstalled && installed != selected branch. When the app wasn't installed at all, that branch was skipped and the cascade fell straight to theelse -> "Install latest"regardless of what the picker had selected.Fix
Insert an intermediate branch that fires when the app isn't installed yet but
state.selectedRelease.tagNamediffers fromstate.allReleases.first().tagName(GitHub returns the feed newest-first bypublished_at, so the head is the latest). When that's the case, renderInstall version <selectedTag>.Install lateststays as the genuine fall-through:allReleases, orallReleasesis empty / we can't compare.Behaviour matrix
Verify
./gradlew :feature:details:presentation:compileDebugKotlinAndroidpasses clean (only pre-existing warnings).Summary by CodeRabbit