feat: Added asset selection for multiple installer variants (#293)#314
Merged
rainxchzed merged 5 commits intoOpenHub-Store:mainfrom Mar 10, 2026
Merged
feat: Added asset selection for multiple installer variants (#293)#314rainxchzed merged 5 commits intoOpenHub-Store:mainfrom
rainxchzed merged 5 commits intoOpenHub-Store:mainfrom
Conversation
Included an assets picker to allow users to select specific download assets Included strings.xml res for the new string resources used no-translations are added though for them
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)
WalkthroughAdds an asset-selection UI and wiring to Details: new ReleaseAssetsPicker component, VersionTypePicker, ViewModel action handlers (SelectDownloadAsset, ToggleReleaseAssetsPicker), new state fields (userProfile, primaryAsset, isReleaseSelectorVisible), VersionPicker signature change, and string resource reordering. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant UI as ReleaseAssetsPicker
participant VM as DetailsViewModel
participant State as DetailsStateStore
User->>UI: Tap to open assets picker
UI->>VM: Dispatch ToggleReleaseAssetsPicker
VM->>State: set isReleaseSelectorVisible = true
State-->>UI: visible = true
UI->>User: show bottom sheet with assets
User->>UI: Select asset A
UI->>VM: Dispatch SelectDownloadAsset(asset A)
VM->>State: set primaryAsset = asset A
State-->>UI: updated primaryAsset
UI->>User: display selected asset in header
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 3
🧹 Nitpick comments (6)
feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/SmartInstallButton.kt (1)
480-487: Consider extractingformatFileSizeto a shared utility.This function is duplicated in
ReleaseAssetsPicker.kt(lines 292-299) with identical implementation. Consider extracting it to a shared utility in theutilspackage to follow DRY principles.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/SmartInstallButton.kt` around lines 480 - 487, Extract the duplicated top-level function formatFileSize into a single shared utility in the utils package (e.g. create a FileSizeUtils.kt or top-level FileSizeUtils function formatFileSize(bytes: Long): String) and remove the duplicate implementations in SmartInstallButton.kt and ReleaseAssetsPicker.kt; update both files to import and call the shared formatFileSize function (preserving the exact behavior/formatting) so both components reference the single utility instead of keeping local copies.feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsAction.kt (1)
55-56: Misleading parameter name inSelectDownloadAsset.The parameter is named
releasebut the type isGithubAsset. This is confusing since elsewhere in this filereleaserefers toGithubRelease(see line 43:SelectRelease(val release: GithubRelease)). Consider renaming for clarity.Suggested fix
- data class SelectDownloadAsset(val release: GithubAsset) : DetailsAction + data class SelectDownloadAsset(val asset: GithubAsset) : DetailsAction🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsAction.kt` around lines 55 - 56, Rename the misleading parameter in the data class SelectDownloadAsset from "release" to "asset" to reflect its GithubAsset type; update the data class declaration (SelectDownloadAsset) and all call sites/usages that instantiate or destructure SelectDownloadAsset to use the new parameter name, and ensure this change remains consistent with the nearby SelectRelease(val release: GithubRelease) declaration to avoid confusion between GithubAsset and GithubRelease.feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/ReleaseAssetsPicker.kt (2)
79-81: Unnecessary use ofderivedStateOffor a simple boolean.
derivedStateOfis designed to avoid recomposition when derived values don't change despite input changes. Here,assetsList.isNotEmpty()is trivially computed and changes only whenassetsListchanges, which already triggers recomposition via therememberkey. A simpleremembersuffices.♻️ Suggested simplification
- val isPickerEnabled by remember(assetsList) { - derivedStateOf { assetsList.isNotEmpty() } - } + val isPickerEnabled = remember(assetsList) { assetsList.isNotEmpty() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/ReleaseAssetsPicker.kt` around lines 79 - 81, The boolean isPickerEnabled uses derivedStateOf unnecessarily; replace the derivedStateOf usage with a simple remember keyed on assetsList that returns assetsList.isNotEmpty() (i.e., change the declaration of isPickerEnabled in ReleaseAssetsPicker to use remember(assetsList) { assetsList.isNotEmpty() }), removing derivedStateOf to simplify and avoid the extra wrapper.
84-90: Bottom sheet rendered unconditionally outside the visible Column layout.The
ReleaseAssetsItemsPickeris invoked before the mainColumnlayout. While it internally checksshowPickerand returns early, this ordering could be confusing for readers. Consider placing it after the main UI or within the Column for better code organization and readability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/ReleaseAssetsPicker.kt` around lines 84 - 90, The ReleaseAssetsItemsPicker is declared before the main Column which makes the layout ordering confusing even though it early-returns based on isPickerVisible; move the ReleaseAssetsItemsPicker invocation to after the main UI Column (or place it inside the Column where appropriate) so the picker is rendered in code order with the visible layout, keeping the same props (showPicker = isPickerVisible, assetsList, selectedAsset, onDismiss = { onAction(DetailsAction.ToggleReleaseAssetsPicker) }, onSelect = { onAction(DetailsAction.SelectDownloadAsset(it)) }) and without changing its internal visibility check.feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/VersionPicker.kt (2)
94-96: Consider usingTextOverflow.Ellipsisinstead ofClipfor better UX.Using
TextOverflow.Clipwill abruptly cut off text without visual indication that content is truncated.TextOverflow.Ellipsis(used elsewhere in this file, e.g., line 105) provides a better user experience by indicating truncation with "...".♻️ Suggested fix
text = selectedRelease?.tagName ?: stringResource(Res.string.no_version_selected), style = MaterialTheme.typography.titleSmall, fontWeight = FontWeight.SemiBold, - overflow = TextOverflow.Clip, + overflow = TextOverflow.Ellipsis, maxLines = 1, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/VersionPicker.kt` around lines 94 - 96, In VersionPicker.kt update the Text composable styling that currently sets overflow = TextOverflow.Clip to use TextOverflow.Ellipsis so truncated version labels show "..." (match the other usage in this file around the existing ellipsis at line 105); locate the Text where fontWeight = FontWeight.SemiBold and maxLines = 1 and replace the overflow value accordingly to improve UX.
62-64: SamederivedStateOfsimplification opportunity.Similar to the feedback for
ReleaseAssetsPicker, thisderivedStateOfusage is unnecessary for a simple boolean check.♻️ Suggested simplification
- val isPickerEnabled by remember(filteredReleases) { - derivedStateOf { filteredReleases.isNotEmpty() } - } + val isPickerEnabled = remember(filteredReleases) { filteredReleases.isNotEmpty() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/VersionPicker.kt` around lines 62 - 64, Replace the unnecessary remember + derivedStateOf pattern for isPickerEnabled in VersionPicker.kt: instead of "val isPickerEnabled by remember(filteredReleases) { derivedStateOf { filteredReleases.isNotEmpty() } }" just compute the boolean directly (e.g., "val isPickerEnabled = filteredReleases.isNotEmpty()"), referencing the isPickerEnabled variable and filteredReleases in the VersionPicker component.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@core/presentation/src/commonMain/composeResources/values/strings.xml`:
- Line 387: The string resource "no_version_selected" currently contains the
placeholder "0" which is inappropriate for user-facing UI; update the value of
the string resource named no_version_selected to a descriptive fallback such as
"No version" (or localized equivalent) so that VersionPicker.kt's fallback when
selectedRelease?.tagName is null displays a proper label; ensure the string name
remains unchanged and adjust any translations if present.
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/AppHeader.kt`:
- Around line 248-263: The derivePlatformsFromAssets function uses a single when
branch which stops after the first match, so multiple platforms (e.g., Android
and Windows) can be missed; update the buildList block in
derivePlatformsFromAssets to use separate conditional checks (multiple if
statements) against names.any { ... } for each platform (Android, Windows,
macOS, Linux) so each matching platform is added independently, ensuring all
detected platforms from release.assets are appended to the list.
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/ReleaseAssetsPicker.kt`:
- Around line 171-173: Replace the hardcoded contentDescription in the Icon call
inside IconButton with a localized stringResource; specifically, add a new
string resource (e.g., info_button_content_description) and update the Icon
invocation in ReleaseAssetsPicker (the Icon inside the IconButton whose onClick
sets showInfoDialog = true) to use
stringResource(R.string.info_button_content_description) instead of the literal
"Info" so it matches the rest of the file's localization approach.
---
Nitpick comments:
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/ReleaseAssetsPicker.kt`:
- Around line 79-81: The boolean isPickerEnabled uses derivedStateOf
unnecessarily; replace the derivedStateOf usage with a simple remember keyed on
assetsList that returns assetsList.isNotEmpty() (i.e., change the declaration of
isPickerEnabled in ReleaseAssetsPicker to use remember(assetsList) {
assetsList.isNotEmpty() }), removing derivedStateOf to simplify and avoid the
extra wrapper.
- Around line 84-90: The ReleaseAssetsItemsPicker is declared before the main
Column which makes the layout ordering confusing even though it early-returns
based on isPickerVisible; move the ReleaseAssetsItemsPicker invocation to after
the main UI Column (or place it inside the Column where appropriate) so the
picker is rendered in code order with the visible layout, keeping the same props
(showPicker = isPickerVisible, assetsList, selectedAsset, onDismiss = {
onAction(DetailsAction.ToggleReleaseAssetsPicker) }, onSelect = {
onAction(DetailsAction.SelectDownloadAsset(it)) }) and without changing its
internal visibility check.
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/SmartInstallButton.kt`:
- Around line 480-487: Extract the duplicated top-level function formatFileSize
into a single shared utility in the utils package (e.g. create a
FileSizeUtils.kt or top-level FileSizeUtils function formatFileSize(bytes:
Long): String) and remove the duplicate implementations in SmartInstallButton.kt
and ReleaseAssetsPicker.kt; update both files to import and call the shared
formatFileSize function (preserving the exact behavior/formatting) so both
components reference the single utility instead of keeping local copies.
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/VersionPicker.kt`:
- Around line 94-96: In VersionPicker.kt update the Text composable styling that
currently sets overflow = TextOverflow.Clip to use TextOverflow.Ellipsis so
truncated version labels show "..." (match the other usage in this file around
the existing ellipsis at line 105); locate the Text where fontWeight =
FontWeight.SemiBold and maxLines = 1 and replace the overflow value accordingly
to improve UX.
- Around line 62-64: Replace the unnecessary remember + derivedStateOf pattern
for isPickerEnabled in VersionPicker.kt: instead of "val isPickerEnabled by
remember(filteredReleases) { derivedStateOf { filteredReleases.isNotEmpty() } }"
just compute the boolean directly (e.g., "val isPickerEnabled =
filteredReleases.isNotEmpty()"), referencing the isPickerEnabled variable and
filteredReleases in the VersionPicker component.
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsAction.kt`:
- Around line 55-56: Rename the misleading parameter in the data class
SelectDownloadAsset from "release" to "asset" to reflect its GithubAsset type;
update the data class declaration (SelectDownloadAsset) and all call
sites/usages that instantiate or destructure SelectDownloadAsset to use the new
parameter name, and ensure this change remains consistent with the nearby
SelectRelease(val release: GithubRelease) declaration to avoid confusion between
GithubAsset and GithubRelease.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d5f6223d-6233-4c7a-a25a-3aa24cf5a993
📒 Files selected for processing (10)
core/presentation/src/commonMain/composeResources/values/strings.xmlfeature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsAction.ktfeature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsState.ktfeature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.ktfeature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/AppHeader.ktfeature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/ReleaseAssetsPicker.ktfeature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/SmartInstallButton.ktfeature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/VersionPicker.ktfeature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/VersionTypePicker.ktfeature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/sections/Header.kt
core/presentation/src/commonMain/composeResources/values/strings.xml
Outdated
Show resolved
Hide resolved
.../presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/AppHeader.kt
Show resolved
Hide resolved
...ion/src/commonMain/kotlin/zed/rainxch/details/presentation/components/ReleaseAssetsPicker.kt
Show resolved
Hide resolved
Member
|
@tuuhin Please could you apply the changes that Coderabbit has suggested? I would be able to merge the PR then. |
derivePlatformsFromAssets in AppHeader.kt use of if statements rather than when Use of string resources for content description in ReleaseAssetsPicker.kt
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/AppHeader.kt (1)
263-316: Reuse the sharedInstallStatusBadgeinstead of maintaining a second copy.This implementation matches
core/presentation/src/commonMain/kotlin/zed/rainxch/core/presentation/components/RepositoryCard.ktclosely. Keeping both versions will drift the next time badge styling or copy changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/AppHeader.kt` around lines 263 - 316, This file defines a duplicate InstallStatusBadge; remove this local function and use the shared InstallStatusBadge from core/presentation instead: delete the InstallStatusBadge(...) declaration in AppHeader.kt, add the appropriate import for the shared InstallStatusBadge, and call InstallStatusBadge(isUpdateAvailable = ..., modifier = ...) wherever the local badge was used so parameter names match (isUpdateAvailable, modifier) and styling/copy is centralized.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/AppHeader.kt`:
- Around line 173-177: The current Text call uses author?.login.toString() which
renders "null" for missing profiles; update the AppHeader composable so the Text
content only shows when author?.login is non-null or substitute a real fallback
(e.g., empty string or "Unknown") before passing to
stringResource(Res.string.by_author, ...). Locate the Text usage in AppHeader
(the Text composable that calls stringResource(Res.string.by_author,
author?.login.toString())) and change it to supply a safe non-null value (guard
the call with author?.login != null or compute val displayLogin = author?.login
?: fallback) so the UI never displays "by null".
---
Nitpick comments:
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/AppHeader.kt`:
- Around line 263-316: This file defines a duplicate InstallStatusBadge; remove
this local function and use the shared InstallStatusBadge from core/presentation
instead: delete the InstallStatusBadge(...) declaration in AppHeader.kt, add the
appropriate import for the shared InstallStatusBadge, and call
InstallStatusBadge(isUpdateAvailable = ..., modifier = ...) wherever the local
badge was used so parameter names match (isUpdateAvailable, modifier) and
styling/copy is centralized.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6dd03c05-9088-4775-ac40-222c06c09dd3
📒 Files selected for processing (3)
core/presentation/src/commonMain/composeResources/values/strings.xmlfeature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/AppHeader.ktfeature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/ReleaseAssetsPicker.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/ReleaseAssetsPicker.kt
- core/presentation/src/commonMain/composeResources/values/strings.xml
.../presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/AppHeader.kt
Outdated
Show resolved
Hide resolved
derivePlatformsFromAssets in AppHeader.kt use of if statements rather than when Use of string resources for content description in ReleaseAssetsPicker.kt
…tHub-Store into feature/assets-options
Member
|
Everything looks good, lets get it merged. @tuuhin Thanks a lot for your contribution ❤️ |
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.
This PR includes an Asset Picker logic. If a release contains multiple installer files, the application will now allow user to select the variant option (#293 )
Key Changes:
Screenshots
Windows
Android
Summary by CodeRabbit
New Features
Refactor