chore: update ktlint.yml#78
Merged
Merged
Conversation
|
Note Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported. |
Contributor
📝 WalkthroughWalkthroughThis PR updates the ivi-demo package and asset naming, refactors lyrics-maker search and project-details test coverage, adds a TemplateData boolean accessor, and applies lint-driven formatting and suppression changes across multiple modules. Changesivi-demo Module Rename and Asset Constants
Lyrics Maker Search and Project Details
TemplateData Boolean Accessor
CI Workflow and IDE Configuration
ktlint-driven Formatting Cleanup
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
Contributor
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
sdui | 90711c5 | Commit Preview URL Branch Preview URL |
Jul 04 2026, 02:29 PM |
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
This PR attempts to update the ktlint reporter configuration but introduces a breaking change. The switch from checkstyle to plain reporter is incompatible with the action-ktlint@v4 action used on line 25-29, which requires XML format output. The plain reporter outputs to console/stdout and does not generate the XML file expected at build/*.xml, causing the workflow to fail. The original checkstyle reporter should be retained to maintain workflow functionality.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
| - name: run ktlint | ||
| run: | | ||
| ktlint --reporter=checkstyle,output=build/ktlint-report.xml | ||
| ktlint --reporter=plain?group_by_file,output=build/ktlint-report.xml |
Contributor
There was a problem hiding this comment.
🛑 CI Pipeline Failure: The plain reporter does not generate XML output, but line 27 expects an XML file at build/*.xml for the action-ktlint@v4 action to parse. This will cause the workflow to fail because the action cannot find or parse the required report file. The checkstyle reporter should be retained as it produces XML output compatible with the action.
Suggested change
| ktlint --reporter=plain?group_by_file,output=build/ktlint-report.xml | |
| ktlint --reporter=checkstyle,output=build/ktlint-report.xml |
Contributor
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
lyrics | 90711c5 | Commit Preview URL Branch Preview URL |
Jul 04 2026, 02:30 PM |
Contributor
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
motion-lib | 90711c5 | Commit Preview URL Branch Preview URL |
Jul 04 2026, 02:31 PM |
tejpratap46
force-pushed
the
chore/fix-ktlint
branch
3 times, most recently
from
c03b784 to
c5054e3
Compare
![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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
modules/ivi-demo/src/androidTest/java/com/tejpratapsingh/ividemo/ExampleInstrumentedTest.kt (1)
17-21: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winTest assertion still expects old applicationId.
applicationIdwas renamed tocom.tejpratapsingh.ividemoin build.gradle, but this assertion still checks for the oldcom.tejpratapsingh.ivi_demostring, so this test will fail.🐛 Proposed fix
- assertEquals("com.tejpratapsingh.ivi_demo", appContext.packageName) + assertEquals("com.tejpratapsingh.ividemo", appContext.packageName)🤖 Prompt for 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. In `@modules/ivi-demo/src/androidTest/java/com/tejpratapsingh/ividemo/ExampleInstrumentedTest.kt` around lines 17 - 21, The instrumented test assertion in ExampleInstrumentedTest.useAppContext still checks the old package name string, so update the expected value to match the renamed applicationId used by the app. Verify the assertion against the current value from build.gradle, keeping the test aligned with com.tejpratapsingh.ividemo.modules/ivi-demo/src/main/java/com/tejpratapsingh/ividemo/MainActivity.kt (1)
6-6: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winUpdate the import path
MainActivity.kt:6still importscom.tejpratapsingh.ivi_demo.extension.enableSwipeSeekReverse; it should becom.tejpratapsingh.ividemo.extension.enableSwipeSeekReverse.🤖 Prompt for 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. In `@modules/ivi-demo/src/main/java/com/tejpratapsingh/ividemo/MainActivity.kt` at line 6, The import in MainActivity should use the updated package name, since it still references the old com.tejpratapsingh.ivi_demo.extension.enableSwipeSeekReverse path. Update the import at the top of MainActivity.kt to com.tejpratapsingh.ividemo.extension.enableSwipeSeekReverse so it matches the current module/package naming and resolves correctly.
🧹 Nitpick comments (2)
.github/workflows/ktlint.yml (1)
30-35: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueConsider
if-no-files-foundfor resilience.With
if: always(), this step runs even if the prior ktlint step crashes before writingbuild/ktlint-report.xml(as opposed to just lint failures, which the checkstyle reporter still writes for). In that edge case,upload-artifactwill fail on the missing file. Settingif-no-files-found: warnavoids masking the real failure with an unrelated artifact error.♻️ Suggested tweak
- name: Upload ktlint report uses: actions/upload-artifact@v4 if: always() with: name: ktlint-report path: build/ktlint-report.xml + if-no-files-found: warn🤖 Prompt for 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. In @.github/workflows/ktlint.yml around lines 30 - 35, The ktlint artifact upload step can fail when build/ktlint-report.xml was never created, because the step runs with if: always(). Update the Upload ktlint report job in the workflow to use actions/upload-artifact@v4 with if-no-files-found set to warn so missing report files do not override the real ktlint failure; keep the existing ktlint-report artifact name and path..idea/deploymentTargetSelector.xml (1)
30-40: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUnrelated IDE local state committed in a "ktlint.yml" chore PR.
This adds a machine-specific device selection (hardcoded serial
192.168.1.22:5555) unrelated to the stated PR scope of updating the ktlint workflow. This file tracks per-developer IDE run state and will keep churning/conflicting across contributors' machines. Consider adding.idea/deploymentTargetSelector.xmlto.gitignoreinstead of committing local device state.🤖 Prompt for 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. In @.idea/deploymentTargetSelector.xml around lines 30 - 40, Remove the machine-specific IDE run state from deploymentTargetSelector.xml, since it belongs to local per-developer configuration rather than the ktlint workflow change. Delete the hardcoded androidTest Target/DeviceId selection (including the serial 192.168.1.22:5555 entry) from the committed diff, and avoid tracking this file in source control by excluding .idea/deploymentTargetSelector.xml so it does not keep churning across machines.
🤖 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
`@modules/ivi-demo/src/main/java/com/tejpratapsingh/ividemo/extension/ViewExtensions.kt`:
- Line 1: MainActivity is still importing the swipe-seek extension from the old
package, so update its import to match the new ViewExtensions package. Use the
existing enableSwipeSeekReverse symbol in MainActivity.kt and switch the import
to com.tejpratapsingh.ividemo.extension so the extension resolves correctly at
compile time.
In
`@modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/search/SearchScreen.kt`:
- Around line 213-219: The trailing icon in SearchScreen’s trailingIcon block is
using the wrong visual affordance for the clear action. Update the IconButton
inside SearchScreen so the clear control uses a clear/close-style icon instead
of Icons.Default.Search, and keep the search icon reserved for the actual search
affordance used elsewhere in the screen. Preserve the existing conditional
display based on query.isNotEmpty() and the viewModel.query.value = "" click
behavior.
- Around line 203-220: The search field in SearchScreen is only updating
viewModel.query and never triggering a search, so wire it to the actual search
flow used by LyricsViewModel. Update the OutlinedTextField handling so that
query changes or an explicit action (such as IME search/submit or a search
button) calls searchLyrics(...) with the current text, and ensure the clear
action also resets state appropriately and triggers the expected refresh/empty
result behavior.
In
`@modules/motionlib/src/main/java/com/tejpratapsingh/motionlib/utils/ImageUtil.kt`:
- Around line 17-49: The URL handling in fetchBitmap/fetchFromNetwork currently
passes SDUI-provided http(s) URIs straight into OkHttp, which allows untrusted
requests to internal or link-local hosts. Add strict validation before
Request.Builder().url(...) by allowlisting only approved schemes and hostnames,
and reject or fall back when the Uri does not match; keep the check close to
fetchFromNetwork so all callers of fetchBitmap are protected.
---
Outside diff comments:
In
`@modules/ivi-demo/src/androidTest/java/com/tejpratapsingh/ividemo/ExampleInstrumentedTest.kt`:
- Around line 17-21: The instrumented test assertion in
ExampleInstrumentedTest.useAppContext still checks the old package name string,
so update the expected value to match the renamed applicationId used by the app.
Verify the assertion against the current value from build.gradle, keeping the
test aligned with com.tejpratapsingh.ividemo.
In `@modules/ivi-demo/src/main/java/com/tejpratapsingh/ividemo/MainActivity.kt`:
- Line 6: The import in MainActivity should use the updated package name, since
it still references the old
com.tejpratapsingh.ivi_demo.extension.enableSwipeSeekReverse path. Update the
import at the top of MainActivity.kt to
com.tejpratapsingh.ividemo.extension.enableSwipeSeekReverse so it matches the
current module/package naming and resolves correctly.
---
Nitpick comments:
In @.github/workflows/ktlint.yml:
- Around line 30-35: The ktlint artifact upload step can fail when
build/ktlint-report.xml was never created, because the step runs with if:
always(). Update the Upload ktlint report job in the workflow to use
actions/upload-artifact@v4 with if-no-files-found set to warn so missing report
files do not override the real ktlint failure; keep the existing ktlint-report
artifact name and path.
In @.idea/deploymentTargetSelector.xml:
- Around line 30-40: Remove the machine-specific IDE run state from
deploymentTargetSelector.xml, since it belongs to local per-developer
configuration rather than the ktlint workflow change. Delete the hardcoded
androidTest Target/DeviceId selection (including the serial 192.168.1.22:5555
entry) from the committed diff, and avoid tracking this file in source control
by excluding .idea/deploymentTargetSelector.xml so it does not keep churning
across machines.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: d56c117c-1d04-4716-a0ca-01e8fa9e8fd0
📒 Files selected for processing (54)
.github/workflows/ktlint.yml.idea/deploymentTargetSelector.xmlmodules/app/src/main/java/com/tejpratapsingh/animator/ui/view/RenaultCar.ktmodules/core/src/main/java/com/tejpratapsingh/motionlib/core/MotionLayoutInfo.ktmodules/core/src/main/java/com/tejpratapsingh/motionlib/core/VideoAspectRatio.ktmodules/core/src/test/java/com/tejpratapsingh/motionlib/core/MotionTextSizeProviderTest.ktmodules/ivi-demo/build.gradlemodules/ivi-demo/src/androidTest/java/com/tejpratapsingh/ividemo/ExampleInstrumentedTest.ktmodules/ivi-demo/src/main/java/com/tejpratapsingh/ividemo/MainActivity.ktmodules/ivi-demo/src/main/java/com/tejpratapsingh/ividemo/extension/ViewExtensions.ktmodules/ivi-demo/src/main/java/com/tejpratapsingh/ividemo/motion/RenaultCar.ktmodules/ivi-demo/src/main/java/com/tejpratapsingh/ividemo/motion/Road.ktmodules/ivi-demo/src/main/java/com/tejpratapsingh/ividemo/sequence/RenaultSequence.ktmodules/ivi-demo/src/main/java/com/tejpratapsingh/ividemo/view/TrapezoidImageView.ktmodules/ivi-demo/src/test/java/com/tejpratapsingh/ividemo/ExampleUnitTest.ktmodules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/common/GradientText.ktmodules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/lyrics/DragHandle.ktmodules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/lyrics/LyricRow.ktmodules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/lyrics/StartDragHandle.ktmodules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/lyrics/SyncedLyricsSelector.ktmodules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/projects/ProjectThumbnailExtractor.ktmodules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/projects/ThumbnailCache.ktmodules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/search/SearchScreen.ktmodules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/templates/LyricsTemplateSelector.ktmodules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/templates/TemplatePreviewItem.ktmodules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/templates/ZoomLyricsTemplate.ktmodules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/viewmodel/LyricsViewModel.ktmodules/lyrics-maker/src/test/java/com/tejpratapsingh/lyricsmaker/presentation/viewmodel/LyricsViewModelTest.ktmodules/media3-motion-ext/src/main/AndroidManifest.xmlmodules/media3-motion-ext/src/main/java/com/tejpratapsingh/motionlib/media3/FrameProcessor.ktmodules/media3-motion-ext/src/main/java/com/tejpratapsingh/motionlib/media3/Media3TransformerRunner.ktmodules/ml-kit-ext/src/main/java/com/tejpratapsingh/motionlib/mlkit/plugins/SubjectSegmentationPlugin.ktmodules/motion-store/src/main/java/com/tejpratapsingh/motionstore/dao/MotionProjectDao.ktmodules/motion-store/src/main/java/com/tejpratapsingh/motionstore/infra/PreferenceManager.ktmodules/motion-video-editor/src/main/java/com/tejpratapsingh/motioneditor/TimelineData.ktmodules/motion-video-editor/src/main/java/com/tejpratapsingh/motioneditor/ui/MotionEditorScreen.ktmodules/motion-video-editor/src/main/java/com/tejpratapsingh/motioneditor/ui/MotionTimeline.ktmodules/motion-video-editor/src/main/java/com/tejpratapsingh/motioneditor/utils/TimelineUtils.ktmodules/motion-video-player/src/main/java/com/tejpratapsingh/motionlib/ui/custom/video/MotionVideoPlayerCompose.ktmodules/motionlib/src/main/java/com/tejpratapsingh/motionlib/core/animation/Spring.ktmodules/motionlib/src/main/java/com/tejpratapsingh/motionlib/ui/effects/BlurEffect.ktmodules/motionlib/src/main/java/com/tejpratapsingh/motionlib/ui/effects/BrightnessContrastEffect.ktmodules/motionlib/src/main/java/com/tejpratapsingh/motionlib/ui/effects/InvertEffect.ktmodules/motionlib/src/main/java/com/tejpratapsingh/motionlib/ui/effects/PixelateEffect.ktmodules/motionlib/src/main/java/com/tejpratapsingh/motionlib/ui/effects/SepiaEffect.ktmodules/motionlib/src/main/java/com/tejpratapsingh/motionlib/utils/ImageUtil.ktmodules/motionlib/src/main/java/com/tejpratapsingh/motionlib/worker/MotionWorker.ktmodules/sdui/src/main/java/com/tejpratapsingh/motion/sdui/infra/MotionSdui.ktmodules/sdui/src/main/java/com/tejpratapsingh/motion/sdui/infra/MotionTransitionParser.ktmodules/sdui/src/main/java/com/tejpratapsingh/motion/sdui/infra/VideoAspectRatioParser.ktmodules/templates/src/androidTest/java/com/tejpratapsingh/motionlib/templates/MotionTemplateInstrumentationTest.ktmodules/templates/src/androidTest/java/com/tejpratapsingh/motionlib/templates/sdui/MotionTemplateSDUIProviderTest.ktmodules/templates/src/main/java/com/tejpratapsingh/motionlib/templates/model/TemplateData.ktmodules/templates/src/main/java/com/tejpratapsingh/motionlib/templates/model/TemplateParameter.kt
💤 Files with no reviewable changes (3)
- modules/media3-motion-ext/src/main/java/com/tejpratapsingh/motionlib/media3/Media3TransformerRunner.kt
- modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/projects/ProjectThumbnailExtractor.kt
- modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/templates/ZoomLyricsTemplate.kt
Comment on lines
203
to
+220
| OutlinedTextField( | ||
| value = query.value, | ||
| onValueChange = { viewModel.query.tryEmit(it) }, | ||
| label = { Text("Search") }, | ||
| singleLine = true, | ||
| value = query, | ||
| onValueChange = { viewModel.query.value = it }, | ||
| modifier = Modifier.fillMaxWidth(), | ||
| placeholder = { Text("Search songs, artists...") }, | ||
| leadingIcon = { | ||
| IconButton(onClick = onBack) { | ||
| Icon(Icons.AutoMirrored.Filled.ArrowBack, contentDescription = "Back") | ||
| } | ||
| }, | ||
| trailingIcon = { | ||
| if (uiState.value is LyricsUiState.Loading) { | ||
| CircularProgressIndicator( | ||
| modifier = Modifier.size(20.dp), | ||
| strokeWidth = 2.dp, | ||
| ) | ||
| if (query.isNotEmpty()) { | ||
| IconButton(onClick = { viewModel.query.value = "" }) { | ||
| Icon(Icons.Default.Search, contentDescription = "Clear") | ||
| } | ||
| } | ||
| }, | ||
| keyboardOptions = KeyboardOptions.Default.copy(imeAction = ImeAction.Search), | ||
| keyboardActions = | ||
| KeyboardActions( | ||
| onSearch = { | ||
| val searchQuery = query.value.trim() | ||
| if (searchQuery.isNotBlank()) { | ||
| keyboardController?.hide() | ||
| RecentSearchHelper.saveSearch(context, searchQuery) | ||
| recentSearches.value = RecentSearchHelper.getSearches(context) | ||
| viewModel.searchLyrics(query = searchQuery) | ||
| } | ||
| }, | ||
| ), | ||
| modifier = Modifier.fillMaxWidth(), | ||
| singleLine = true, |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check whether LyricsViewModel observes `query` to trigger searchLyrics automatically.
rg -n -A5 -B5 'init \{|query\.' modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/viewmodel/LyricsViewModel.ktRepository: tejpratap46/AndroidVideoMotion
Length of output: 168
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the viewmodel and inspect its structure.
fd -a 'LyricsViewModel.kt' modules
printf '\n--- outline ---\n'
ast-grep outline modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/viewmodel/LyricsViewModel.kt --view expanded || true
printf '\n--- search for query/searchLyrics references ---\n'
rg -n -A4 -B4 'query|searchLyrics|debounce|collectLatest|init \{' modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/viewmodel/LyricsViewModel.kt || trueRepository: tejpratap46/AndroidVideoMotion
Length of output: 2860
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the search screen around the text field and any side effects tied to query changes.
ast-grep outline modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/search/SearchScreen.kt --view expanded || true
printf '\n--- relevant lines ---\n'
sed -n '1,320p' modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/search/SearchScreen.kt | nl -ba | sed -n '1,320p'Repository: tejpratap46/AndroidVideoMotion
Length of output: 386
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/search/SearchScreen.kt
printf '%s\n' '--- query references in SearchScreen.kt ---'
rg -n -A4 -B4 'query|searchLyrics|onSearch|LaunchedEffect|collectLatest|debounce' "$FILE" || true
printf '\n%s\n' '--- numbered excerpt around the text field ---'
awk 'NR>=180 && NR<=240 { printf "%d:%s\n", NR, $0 }' "$FILE"Repository: tejpratap46/AndroidVideoMotion
Length of output: 3607
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- searchLyrics usages ---'
rg -n 'searchLyrics\s*\(' modules/lyrics-maker/src/main/java || true
printf '\n%s\n' '--- query state usages ---'
rg -n 'viewModel\.query|query\.collectAsState|MutableStateFlow<String>\s*=\s*MutableStateFlow\(""\)' modules/lyrics-maker/src/main/java || trueRepository: tejpratap46/AndroidVideoMotion
Length of output: 2226
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/activity/SearchActivity.kt
printf '%s\n' '--- SearchActivity excerpt ---'
awk 'NR>=120 && NR<=240 { printf "%d:%s\n", NR, $0 }' "$FILE"Repository: tejpratap46/AndroidVideoMotion
Length of output: 4157
Wire the search field to an actual search trigger. onValueChange only updates viewModel.query; nothing here calls searchLyrics(...), and LyricsViewModel doesn’t auto-react to query, so typing or clearing this field won’t start a search. SearchScreen.kt:203-220
🤖 Prompt for 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.
In
`@modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/search/SearchScreen.kt`
around lines 203 - 220, The search field in SearchScreen is only updating
viewModel.query and never triggering a search, so wire it to the actual search
flow used by LyricsViewModel. Update the OutlinedTextField handling so that
query changes or an explicit action (such as IME search/submit or a search
button) calls searchLyrics(...) with the current text, and ensure the clear
action also resets state appropriately and triggers the expected refresh/empty
result behavior.
Comment on lines
213
to
219
| trailingIcon = { | ||
| if (uiState.value is LyricsUiState.Loading) { | ||
| CircularProgressIndicator( | ||
| modifier = Modifier.size(20.dp), | ||
| strokeWidth = 2.dp, | ||
| ) | ||
| if (query.isNotEmpty()) { | ||
| IconButton(onClick = { viewModel.query.value = "" }) { | ||
| Icon(Icons.Default.Search, contentDescription = "Clear") | ||
| } | ||
| } | ||
| }, |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Trailing "Clear" icon shows a magnifying glass, not a clear icon.
Icons.Default.Search is used for the clear button while contentDescription says "Clear" — visually confusing since the same icon (search) is also used elsewhere (Line 162) for its intended meaning.
🔧 Proposed fix
trailingIcon = {
if (query.isNotEmpty()) {
IconButton(onClick = { viewModel.query.value = "" }) {
- Icon(Icons.Default.Search, contentDescription = "Clear")
+ Icon(Icons.Default.Close, contentDescription = "Clear")
}
}
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| trailingIcon = { | |
| if (uiState.value is LyricsUiState.Loading) { | |
| CircularProgressIndicator( | |
| modifier = Modifier.size(20.dp), | |
| strokeWidth = 2.dp, | |
| ) | |
| if (query.isNotEmpty()) { | |
| IconButton(onClick = { viewModel.query.value = "" }) { | |
| Icon(Icons.Default.Search, contentDescription = "Clear") | |
| } | |
| } | |
| }, | |
| trailingIcon = { | |
| if (query.isNotEmpty()) { | |
| IconButton(onClick = { viewModel.query.value = "" }) { | |
| Icon(Icons.Default.Close, contentDescription = "Clear") | |
| } | |
| } | |
| }, |
🤖 Prompt for 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.
In
`@modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/search/SearchScreen.kt`
around lines 213 - 219, The trailing icon in SearchScreen’s trailingIcon block
is using the wrong visual affordance for the clear action. Update the IconButton
inside SearchScreen so the clear control uses a clear/close-style icon instead
of Icons.Default.Search, and keep the search icon reserved for the actual search
affordance used elsewhere in the screen. Preserve the existing conditional
display based on query.isNotEmpty() and the viewModel.query.value = "" click
behavior.
Comment on lines
+17
to
49
| suspend fun fetchBitmap( | ||
| context: Context, | ||
| uri: Uri, | ||
| ): Bitmap? = | ||
| withContext(Dispatchers.IO) { | ||
| val bitmap = | ||
| when (uri.scheme) { | ||
| "http", "https" -> fetchFromNetwork(uri.toString()) | ||
| "content", "file", "android.resource" -> fetchFromLocal(context, uri) | ||
| else -> null | ||
| } | ||
| return@withContext bitmap ?: fetchDefault(context) | ||
| } | ||
| return@withContext bitmap ?: fetchDefault(context) | ||
| } | ||
|
|
||
| private fun fetchDefault(context: Context): Bitmap? { | ||
| return try { | ||
| private fun fetchDefault(context: Context): Bitmap? = | ||
| try { | ||
| BitmapFactory.decodeResource(context.resources, R.drawable.default_bg) | ||
| } catch (e: Exception) { | ||
| null | ||
| } | ||
| } | ||
|
|
||
| private fun fetchFromNetwork(url: String): Bitmap? { | ||
| return try { | ||
| if (url.isBlank()) return null | ||
| val request = Request.Builder().url(url).build() | ||
| client.newCall(request).execute().use { response -> | ||
| if (!response.isSuccessful) return null | ||
| val bytes = response.body()?.bytes() ?: return null | ||
| BitmapFactory.decodeByteArray(bytes, 0, bytes.size) | ||
| private fun fetchFromNetwork(url: String): Bitmap? = | ||
| try { | ||
| if (url.isBlank()) { | ||
| null | ||
| } else { | ||
| val request = Request.Builder().url(url).build() | ||
| client.newCall(request).execute().use { response -> | ||
| if (!response.isSuccessful) return null | ||
| val bytes = response.body()?.bytes() ?: return null | ||
| BitmapFactory.decodeByteArray(bytes, 0, bytes.size) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target file with line numbers
sed -n '1,220p' modules/motionlib/src/main/java/com/tejpratapsingh/motionlib/utils/ImageUtil.kt | cat -n
# Find call sites and related URI handling
rg -n "fetchBitmap\(|fetchFromNetwork\(|ImageUtil|Uri\.parse|scheme" modules -g '!**/build/**'Repository: tejpratap46/AndroidVideoMotion
Length of output: 7852
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the custom views that call ImageUtil.fetchBitmap
sed -n '1,220p' modules/motionlib/src/main/java/com/tejpratapsingh/motionlib/ui/custom/image/MotionImageView.kt | cat -n
printf '\n---\n'
sed -n '1,220p' modules/motionlib/src/main/java/com/tejpratapsingh/motionlib/ui/custom/image/CircularMotionImageView.kt | cat -n
# Find where imageUri is populated
printf '\n=== imageUri references ===\n'
rg -n "imageUri|setImageUri|src=|motionImage|circularMotion" modules/motionlib modules/templates -g '!**/build/**'Repository: tejpratap46/AndroidVideoMotion
Length of output: 7644
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect template helpers and example usage around MotionImageView / CircularMotionImageView
sed -n '1,220p' modules/templates/src/main/java/com/tejpratapsingh/motionlib/templates/extensions/ImageExtensions.kt | cat -n
printf '\n---\n'
rg -n "MotionImageView\(|CircularMotionImageView\(|motionImageView\(|circularMotionImageView\(" . -g '!**/build/**' -g '!**/.gradle/**'
# Look for any docs describing accepted URI sources or security constraints
printf '\n=== docs / readme mentions ===\n'
rg -n "Uri|http|https|content|file|android.resource|imageUri|motion image" README* docs* modules -g '!**/build/**'Repository: tejpratap46/AndroidVideoMotion
Length of output: 50387
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the SDUI initializer branches that create image views
sed -n '240,330p' modules/sdui/src/main/java/com/tejpratapsingh/motion/sdui/infra/MotionSduiInitializer.kt | cat -n
# Inspect template README sections that describe image/template inputs
printf '\n=== templates README image sections ===\n'
rg -n -C 3 "motionImageView|circularMotionImageView|imageUri|Uri|content \{|server|JSON" modules/templates/README.mdRepository: tejpratap46/AndroidVideoMotion
Length of output: 7794
Validate the image URL before fetching it.
imageUri can come from SDUI JSON and reaches OkHttp unchanged, so a crafted http(s) value can trigger requests to internal or link-local hosts. Add strict scheme/host allowlisting before Request.Builder().url(...).
🧰 Tools
🪛 ast-grep (0.44.0)
[error] 42-42: An outbound HTTP request is built from a non-literal URL: URL(...).openConnection(), OkHttp's Request.Builder().url(...), or a Spring RestTemplate call (getForObject/getForEntity/postForObject/postForEntity/exchange/execute). When the URL is derived from a variable, method call, concatenation, or interpolation, an attacker can point the request at internal services, cloud metadata endpoints (e.g. 169.254.169.254), or arbitrary hosts (Server-Side Request Forgery). Do not build request targets from untrusted input: validate the URL against a strict allowlist of permitted hosts/schemes, resolve and re-check the host after DNS resolution, and reject internal/link-local/loopback addresses before connecting.
Context: Request.Builder().url(url)
Note: [CWE-918] Server-Side Request Forgery (SSRF).
(ssrf-outbound-request-tainted-url-kotlin)
🤖 Prompt for 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.
In
`@modules/motionlib/src/main/java/com/tejpratapsingh/motionlib/utils/ImageUtil.kt`
around lines 17 - 49, The URL handling in fetchBitmap/fetchFromNetwork currently
passes SDUI-provided http(s) URIs straight into OkHttp, which allows untrusted
requests to internal or link-local hosts. Add strict validation before
Request.Builder().url(...) by allowlisting only approved schemes and hostnames,
and reject or fall back when the Uri does not match; keep the check close to
fetchFromNetwork so all callers of fetchBitmap are protected.
Source: Linters/SAST tools
tejpratap46
force-pushed
the
chore/fix-ktlint
branch
from
c5054e3 to
e9d3647
Compare
tejpratap46
force-pushed
the
chore/fix-ktlint
branch
from
e9d3647 to
90711c5
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
modules/motionlib/src/main/java/com/tejpratapsingh/motionlib/utils/ImageUtil.kt (1)
38-53: 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick winBlock outbound requests until the URL is validated.
fetchFromNetworkstill handsurldirectly toRequest.Builder().url(...), so any caller that reaches this path can steer OkHttp toward arbitrary hosts, including internal or link-local targets. Please add a strict scheme/host allowlist before building the request.🔎 Verification script
#!/bin/bash rg -n "fetchBitmap\(|fetchFromNetwork\(|Request\.Builder\(\)\.url\(" modules/motionlib modules/sdui modules/templates -g '!**/build/**'🤖 Prompt for 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. In `@modules/motionlib/src/main/java/com/tejpratapsingh/motionlib/utils/ImageUtil.kt` around lines 38 - 53, The fetchFromNetwork path in ImageUtil currently passes untrusted url values straight into Request.Builder().url(...), so add a strict preflight validation step before building the request. Update fetchFromNetwork (or the caller chain via fetchBitmap) to enforce an allowlist for scheme and host, reject anything else early, and only construct the OkHttp Request after validation succeeds; keep the existing null-on-failure behavior.Source: Linters/SAST tools
🤖 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
`@modules/lyrics-maker/src/androidTest/java/com/tejpratapsingh/lyricsmaker/presentation/compose/ProjectDetailsScreenTest.kt`:
- Around line 203-242: The Edit action in ProjectDetailsScreen is an icon-only
IconButton with contentDescription "Edit", so the current tests are matching the
wrong semantics. Update both editButton_isDisplayed and
editButton_invokesOnEditClickWithProject in ProjectDetailsScreenTest to locate
the node with onNodeWithContentDescription("Edit") instead of
onNodeWithText("Edit Project"), then keep the display and click assertions
against that node.
---
Duplicate comments:
In
`@modules/motionlib/src/main/java/com/tejpratapsingh/motionlib/utils/ImageUtil.kt`:
- Around line 38-53: The fetchFromNetwork path in ImageUtil currently passes
untrusted url values straight into Request.Builder().url(...), so add a strict
preflight validation step before building the request. Update fetchFromNetwork
(or the caller chain via fetchBitmap) to enforce an allowlist for scheme and
host, reject anything else early, and only construct the OkHttp Request after
validation succeeds; keep the existing null-on-failure behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4e3b5c2e-edca-4a27-b1ed-8a22068056c8
📒 Files selected for processing (84)
.editorconfig.github/workflows/ktlint.yml.idea/deploymentTargetSelector.xmlmodules/app/src/main/java/com/tejpratapsingh/animator/ui/view/RenaultCar.ktmodules/core/src/main/java/com/tejpratapsingh/motionlib/core/MotionLayoutInfo.ktmodules/core/src/main/java/com/tejpratapsingh/motionlib/core/VideoAspectRatio.ktmodules/core/src/test/java/com/tejpratapsingh/motionlib/core/MotionTextSizeProviderTest.ktmodules/ivi-demo/build.gradlemodules/ivi-demo/src/androidTest/java/com/tejpratapsingh/ividemo/ExampleInstrumentedTest.ktmodules/ivi-demo/src/main/java/com/tejpratapsingh/ividemo/MainActivity.ktmodules/ivi-demo/src/main/java/com/tejpratapsingh/ividemo/extension/ViewExtensions.ktmodules/ivi-demo/src/main/java/com/tejpratapsingh/ividemo/motion/RenaultCar.ktmodules/ivi-demo/src/main/java/com/tejpratapsingh/ividemo/motion/Road.ktmodules/ivi-demo/src/main/java/com/tejpratapsingh/ividemo/sequence/RenaultSequence.ktmodules/ivi-demo/src/main/java/com/tejpratapsingh/ividemo/view/TrapezoidImageView.ktmodules/ivi-demo/src/test/java/com/tejpratapsingh/ividemo/ExampleUnitTest.ktmodules/lyrics-maker/src/androidTest/java/com/tejpratapsingh/lyricsmaker/presentation/compose/ProjectDetailsScreenTest.ktmodules/lyrics-maker/src/androidTest/java/com/tejpratapsingh/lyricsmaker/presentation/worker/LyricsMotionWorkerTest.ktmodules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/common/GradientText.ktmodules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/common/GradientTextPreview.ktmodules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/details/ProjectDetailsScreen.ktmodules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/lyrics/DragHandle.ktmodules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/lyrics/LyricRow.ktmodules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/lyrics/StartDragHandle.ktmodules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/lyrics/SyncedLyricsSelector.ktmodules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/navigation/AppNavHost.ktmodules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/projects/CreateNewProjectCard.ktmodules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/projects/DeleteConfirmationDialog.ktmodules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/projects/ProjectCard.ktmodules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/projects/ProjectThumbnailExtractor.ktmodules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/projects/ProjectsRoute.ktmodules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/projects/ProjectsScreen.ktmodules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/projects/ThumbnailCache.ktmodules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/search/SearchScreen.ktmodules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/templates/LyricsTemplateSelector.ktmodules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/templates/TemplatePreviewItem.ktmodules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/templates/ZoomLyricsTemplate.ktmodules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/ui/theme/Theme.ktmodules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/viewmodel/LyricsViewModel.ktmodules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/worker/LyricsMotionWorkerCancelReceiver.ktmodules/lyrics-maker/src/test/java/com/tejpratapsingh/lyricsmaker/presentation/compose/ScreenTest.ktmodules/lyrics-maker/src/test/java/com/tejpratapsingh/lyricsmaker/presentation/viewmodel/LyricsViewModelTest.ktmodules/media3-motion-ext/src/main/AndroidManifest.xmlmodules/media3-motion-ext/src/main/java/com/tejpratapsingh/motionlib/media3/FrameProcessor.ktmodules/media3-motion-ext/src/main/java/com/tejpratapsingh/motionlib/media3/Media3CompositionBuilder.ktmodules/media3-motion-ext/src/main/java/com/tejpratapsingh/motionlib/media3/Media3TransformerRunner.ktmodules/media3-motion-ext/src/main/java/com/tejpratapsingh/motionlib/media3/Media3VideoProducerAdapter.ktmodules/ml-kit-ext/src/main/java/com/tejpratapsingh/motionlib/mlkit/plugins/SubjectSegmentationPlugin.ktmodules/motion-store/src/main/java/com/tejpratapsingh/motionstore/dao/MotionProjectDao.ktmodules/motion-store/src/main/java/com/tejpratapsingh/motionstore/infra/PreferenceManager.ktmodules/motion-video-editor/src/main/java/com/tejpratapsingh/motioneditor/TimelineData.ktmodules/motion-video-editor/src/main/java/com/tejpratapsingh/motioneditor/ui/MotionEditorScreen.ktmodules/motion-video-editor/src/main/java/com/tejpratapsingh/motioneditor/ui/MotionTimeline.ktmodules/motion-video-editor/src/main/java/com/tejpratapsingh/motioneditor/utils/TimelineUtils.ktmodules/motion-video-player/src/main/java/com/tejpratapsingh/motionlib/ui/custom/video/MotionVideoPlayerCompose.ktmodules/motionlib/src/main/java/com/tejpratapsingh/motionlib/core/animation/Spring.ktmodules/motionlib/src/main/java/com/tejpratapsingh/motionlib/core/motion/MotionVideoProducer.ktmodules/motionlib/src/main/java/com/tejpratapsingh/motionlib/ui/custom/text/abstract/AbstractMotionTextView.ktmodules/motionlib/src/main/java/com/tejpratapsingh/motionlib/ui/effects/BlurEffect.ktmodules/motionlib/src/main/java/com/tejpratapsingh/motionlib/ui/effects/BrightnessContrastEffect.ktmodules/motionlib/src/main/java/com/tejpratapsingh/motionlib/ui/effects/ChainEffect.ktmodules/motionlib/src/main/java/com/tejpratapsingh/motionlib/ui/effects/FadeInEffect.ktmodules/motionlib/src/main/java/com/tejpratapsingh/motionlib/ui/effects/FadeOutEffect.ktmodules/motionlib/src/main/java/com/tejpratapsingh/motionlib/ui/effects/GrayscaleEffect.ktmodules/motionlib/src/main/java/com/tejpratapsingh/motionlib/ui/effects/InvertEffect.ktmodules/motionlib/src/main/java/com/tejpratapsingh/motionlib/ui/effects/OffsetEffect.ktmodules/motionlib/src/main/java/com/tejpratapsingh/motionlib/ui/effects/PixelateEffect.ktmodules/motionlib/src/main/java/com/tejpratapsingh/motionlib/ui/effects/SepiaEffect.ktmodules/motionlib/src/main/java/com/tejpratapsingh/motionlib/ui/effects/VibrateEffect.ktmodules/motionlib/src/main/java/com/tejpratapsingh/motionlib/ui/effects/VintageEffect.ktmodules/motionlib/src/main/java/com/tejpratapsingh/motionlib/utils/ImageUtil.ktmodules/motionlib/src/main/java/com/tejpratapsingh/motionlib/worker/MotionWorker.ktmodules/motionlib/src/test/java/com/tejpratapsingh/motionlib/core/motion/MotionTransitionTest.ktmodules/sdui/src/main/java/com/tejpratapsingh/motion/sdui/infra/MotionSDUIParsers.ktmodules/sdui/src/main/java/com/tejpratapsingh/motion/sdui/infra/MotionSdui.ktmodules/sdui/src/main/java/com/tejpratapsingh/motion/sdui/infra/MotionTransitionParser.ktmodules/sdui/src/main/java/com/tejpratapsingh/motion/sdui/infra/VideoAspectRatioParser.ktmodules/templates/src/androidTest/java/com/tejpratapsingh/motionlib/templates/MotionTemplateInstrumentationTest.ktmodules/templates/src/androidTest/java/com/tejpratapsingh/motionlib/templates/sdui/MotionTemplateSDUIProviderTest.ktmodules/templates/src/main/java/com/tejpratapsingh/motionlib/templates/json/JsonMotionTemplate.ktmodules/templates/src/main/java/com/tejpratapsingh/motionlib/templates/model/MotionTemplate.ktmodules/templates/src/main/java/com/tejpratapsingh/motionlib/templates/model/TemplateData.ktmodules/templates/src/main/java/com/tejpratapsingh/motionlib/templates/model/TemplateParameter.ktmodules/templates/src/test/java/com/tejpratapsingh/motionlib/templates/TemplateSerializationTest.kt
💤 Files with no reviewable changes (4)
- modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/templates/ZoomLyricsTemplate.kt
- modules/media3-motion-ext/src/main/java/com/tejpratapsingh/motionlib/media3/Media3TransformerRunner.kt
- modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/projects/ProjectThumbnailExtractor.kt
- modules/motionlib/src/main/java/com/tejpratapsingh/motionlib/ui/custom/text/abstract/AbstractMotionTextView.kt
✅ Files skipped from review due to trivial changes (67)
- modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/lyrics/DragHandle.kt
- modules/core/src/main/java/com/tejpratapsingh/motionlib/core/VideoAspectRatio.kt
- .editorconfig
- modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/common/GradientText.kt
- modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/lyrics/LyricRow.kt
- .idea/deploymentTargetSelector.xml
- modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/common/GradientTextPreview.kt
- modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/projects/CreateNewProjectCard.kt
- modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/projects/ProjectsRoute.kt
- modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/projects/ProjectCard.kt
- modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/details/ProjectDetailsScreen.kt
- modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/projects/ProjectsScreen.kt
- modules/media3-motion-ext/src/main/AndroidManifest.xml
- modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/ui/theme/Theme.kt
- modules/motionlib/src/main/java/com/tejpratapsingh/motionlib/worker/MotionWorker.kt
- modules/templates/src/androidTest/java/com/tejpratapsingh/motionlib/templates/sdui/MotionTemplateSDUIProviderTest.kt
- modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/navigation/AppNavHost.kt
- .github/workflows/ktlint.yml
- modules/motion-video-editor/src/main/java/com/tejpratapsingh/motioneditor/utils/TimelineUtils.kt
- modules/ivi-demo/src/main/java/com/tejpratapsingh/ividemo/sequence/RenaultSequence.kt
- modules/motionlib/src/main/java/com/tejpratapsingh/motionlib/ui/effects/OffsetEffect.kt
- modules/sdui/src/main/java/com/tejpratapsingh/motion/sdui/infra/MotionSDUIParsers.kt
- modules/motionlib/src/main/java/com/tejpratapsingh/motionlib/ui/effects/FadeOutEffect.kt
- modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/worker/LyricsMotionWorkerCancelReceiver.kt
- modules/motionlib/src/main/java/com/tejpratapsingh/motionlib/ui/effects/VintageEffect.kt
- modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/projects/DeleteConfirmationDialog.kt
- modules/motionlib/src/main/java/com/tejpratapsingh/motionlib/ui/effects/GrayscaleEffect.kt
- modules/motionlib/src/main/java/com/tejpratapsingh/motionlib/ui/effects/InvertEffect.kt
- modules/motionlib/src/main/java/com/tejpratapsingh/motionlib/core/animation/Spring.kt
- modules/motion-store/src/main/java/com/tejpratapsingh/motionstore/infra/PreferenceManager.kt
- modules/ivi-demo/src/main/java/com/tejpratapsingh/ividemo/view/TrapezoidImageView.kt
- modules/templates/src/test/java/com/tejpratapsingh/motionlib/templates/TemplateSerializationTest.kt
- modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/lyrics/StartDragHandle.kt
- modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/templates/TemplatePreviewItem.kt
- modules/motionlib/src/main/java/com/tejpratapsingh/motionlib/ui/effects/BrightnessContrastEffect.kt
- modules/motionlib/src/main/java/com/tejpratapsingh/motionlib/ui/effects/SepiaEffect.kt
- modules/templates/src/main/java/com/tejpratapsingh/motionlib/templates/model/TemplateParameter.kt
- modules/motion-video-player/src/main/java/com/tejpratapsingh/motionlib/ui/custom/video/MotionVideoPlayerCompose.kt
- modules/motion-video-editor/src/main/java/com/tejpratapsingh/motioneditor/ui/MotionEditorScreen.kt
- modules/motionlib/src/main/java/com/tejpratapsingh/motionlib/ui/effects/ChainEffect.kt
- modules/ml-kit-ext/src/main/java/com/tejpratapsingh/motionlib/mlkit/plugins/SubjectSegmentationPlugin.kt
- modules/motion-store/src/main/java/com/tejpratapsingh/motionstore/dao/MotionProjectDao.kt
- modules/templates/src/main/java/com/tejpratapsingh/motionlib/templates/model/MotionTemplate.kt
- modules/templates/src/androidTest/java/com/tejpratapsingh/motionlib/templates/MotionTemplateInstrumentationTest.kt
- modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/viewmodel/LyricsViewModel.kt
- modules/core/src/test/java/com/tejpratapsingh/motionlib/core/MotionTextSizeProviderTest.kt
- modules/app/src/main/java/com/tejpratapsingh/animator/ui/view/RenaultCar.kt
- modules/motionlib/src/main/java/com/tejpratapsingh/motionlib/ui/effects/FadeInEffect.kt
- modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/templates/LyricsTemplateSelector.kt
- modules/motionlib/src/main/java/com/tejpratapsingh/motionlib/ui/effects/BlurEffect.kt
- modules/motionlib/src/main/java/com/tejpratapsingh/motionlib/ui/effects/VibrateEffect.kt
- modules/ivi-demo/src/test/java/com/tejpratapsingh/ividemo/ExampleUnitTest.kt
- modules/motionlib/src/main/java/com/tejpratapsingh/motionlib/core/motion/MotionVideoProducer.kt
- modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/projects/ThumbnailCache.kt
- modules/lyrics-maker/src/test/java/com/tejpratapsingh/lyricsmaker/presentation/compose/ScreenTest.kt
- modules/core/src/main/java/com/tejpratapsingh/motionlib/core/MotionLayoutInfo.kt
- modules/sdui/src/main/java/com/tejpratapsingh/motion/sdui/infra/MotionTransitionParser.kt
- modules/motionlib/src/test/java/com/tejpratapsingh/motionlib/core/motion/MotionTransitionTest.kt
- modules/lyrics-maker/src/androidTest/java/com/tejpratapsingh/lyricsmaker/presentation/worker/LyricsMotionWorkerTest.kt
- modules/templates/src/main/java/com/tejpratapsingh/motionlib/templates/json/JsonMotionTemplate.kt
- modules/media3-motion-ext/src/main/java/com/tejpratapsingh/motionlib/media3/Media3VideoProducerAdapter.kt
- modules/motion-video-editor/src/main/java/com/tejpratapsingh/motioneditor/ui/MotionTimeline.kt
- modules/media3-motion-ext/src/main/java/com/tejpratapsingh/motionlib/media3/FrameProcessor.kt
- modules/sdui/src/main/java/com/tejpratapsingh/motion/sdui/infra/MotionSdui.kt
- modules/lyrics-maker/src/test/java/com/tejpratapsingh/lyricsmaker/presentation/viewmodel/LyricsViewModelTest.kt
- modules/motion-video-editor/src/main/java/com/tejpratapsingh/motioneditor/TimelineData.kt
- modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/lyrics/SyncedLyricsSelector.kt
🚧 Files skipped from review as they are similar to previous changes (10)
- modules/ivi-demo/src/main/java/com/tejpratapsingh/ividemo/extension/ViewExtensions.kt
- modules/ivi-demo/src/main/java/com/tejpratapsingh/ividemo/MainActivity.kt
- modules/ivi-demo/src/main/java/com/tejpratapsingh/ividemo/motion/Road.kt
- modules/ivi-demo/build.gradle
- modules/ivi-demo/src/androidTest/java/com/tejpratapsingh/ividemo/ExampleInstrumentedTest.kt
- modules/sdui/src/main/java/com/tejpratapsingh/motion/sdui/infra/VideoAspectRatioParser.kt
- modules/motionlib/src/main/java/com/tejpratapsingh/motionlib/ui/effects/PixelateEffect.kt
- modules/ivi-demo/src/main/java/com/tejpratapsingh/ividemo/motion/RenaultCar.kt
- modules/templates/src/main/java/com/tejpratapsingh/motionlib/templates/model/TemplateData.kt
- modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/search/SearchScreen.kt
Comment on lines
+203
to
+242
|
|
||
| // ------------------------------------------------------------------ | ||
| // Edit button | ||
| // ------------------------------------------------------------------ | ||
|
|
||
| @Test | ||
| fun editButton_isDisplayed() { | ||
| val project = buildProject() | ||
|
|
||
| composeTestRule.setContent { | ||
| ProjectDetailsScreen( | ||
| project = project, | ||
| onBackClick = {}, | ||
| onEditClick = {}, | ||
| onShareClick = {}, | ||
| ) | ||
| } | ||
|
|
||
| composeTestRule.onNodeWithText("Edit Project").assertIsDisplayed() | ||
| } | ||
|
|
||
| @Test | ||
| fun editButton_invokesOnEditClickWithProject() { | ||
| val project = buildProject(id = "edit-test") | ||
| var editedProject: MotionProject? = null | ||
|
|
||
| composeTestRule.setContent { | ||
| ProjectDetailsScreen( | ||
| project = project, | ||
| onBackClick = {}, | ||
| onEditClick = { editedProject = it }, | ||
| onShareClick = {}, | ||
| ) | ||
| } | ||
|
|
||
| composeTestRule.onNodeWithText("Edit Project").performClick() | ||
|
|
||
| assertEquals(project.id, editedProject?.id) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how the Edit button is actually rendered (Text vs contentDescription-only Icon)
rg -n -A5 -B15 'onEditClick\(project\)' modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/details/ProjectDetailsScreen.kt
rg -n '"Edit Project"' modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/details/ProjectDetailsScreen.ktRepository: tejpratap46/AndroidVideoMotion
Length of output: 1033
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="modules/lyrics-maker/src/main/java/com/tejpratapsingh/lyricsmaker/presentation/compose/details/ProjectDetailsScreen.kt"
# Show the Edit button block with line numbers
sed -n '196,225p' "$file" | cat -n
# Also show any nearby text labels/semantics for the screen
rg -n '"Edit Project"|"Back"|contentDescription|Text\(' "$file"Repository: tejpratap46/AndroidVideoMotion
Length of output: 1289
Use the correct semantics matcher for the Edit button
ProjectDetailsScreen renders the action as an icon-only IconButton with contentDescription = "Edit", so onNodeWithText("Edit Project") will never match. Switch both assertions to onNodeWithContentDescription("Edit").
🤖 Prompt for 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.
In
`@modules/lyrics-maker/src/androidTest/java/com/tejpratapsingh/lyricsmaker/presentation/compose/ProjectDetailsScreenTest.kt`
around lines 203 - 242, The Edit action in ProjectDetailsScreen is an icon-only
IconButton with contentDescription "Edit", so the current tests are matching the
wrong semantics. Update both editButton_isDisplayed and
editButton_invokesOnEditClickWithProject in ProjectDetailsScreenTest to locate
the node with onNodeWithContentDescription("Edit") instead of
onNodeWithText("Edit Project"), then keep the display and click assertions
against that node.
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
1 participant
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.
Summary by CodeRabbit