Major rewrite of the Frameo installation instructions#593
Open
Torstein-Eide wants to merge 4 commits intoimmichFrame:mainfrom
Open
Major rewrite of the Frameo installation instructions#593Torstein-Eide wants to merge 4 commits intoimmichFrame:mainfrom
Torstein-Eide wants to merge 4 commits intoimmichFrame:mainfrom
Conversation
📝 WalkthroughWalkthroughExpanded and restructured the Frameo and Frameo WebView setup documentation in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 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}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In `@docs/docs/getting-started/apps.md`:
- Line 223: The markdown image tag currently has no alt text for
Frameo_digital_photo_frames_webview.png; update the tag
 to include a concise, descriptive
alt string (for example: ) so screen readers and cases
where the image fails to load have meaningful text.
- Line 197: Replace the unhyphenated compound adjective "auto complete" with the
hyphenated form "auto-complete" in the sentence containing "`adb push
/path/to/webview.apk /sdcard/webview.apk`" (the line that currently reads
"Android does not have auto complete...") so the documentation uses the correct
compound adjective form.
- Line 133: The markdown image tag
"" is missing alt text; update
that image reference to include a descriptive alt string (for example "Frameo
digital photo frame default app") so screen readers and broken-image contexts
have meaningful text, i.e., modify the markdown image tag to include the alt
text between the brackets.
- Around line 182-184: The fenced code block containing the adb uninstall
command lacks a language identifier; update the block around the line "adb
uninstall com.immichframe.immichframe" to use a shell/bash language tag (e.g.,
```bash) so the snippet is syntax-highlighted and more readable in docs.
- Around line 82-88: Correct typographical errors in the Webview feature note:
change the heading sentence "Webview provieds the following features" to
"Webview provides the following features", change the list item "Video playback
(experimenta)" to "Video playback (experimental)", and change the trailing
"etc)" to "etc.)" so the feature list text is spelled and punctuated correctly.
- Around line 227-229: Fix the blockquote spacing in the fenced code block:
remove the extra spaces after the blockquote symbol on the lines that currently
read as quoted shell lines (the lines wrapping "```shell", "adb shell pm path
com.android.webview", and "```") so each line uses a single ">" followed
immediately by the content (i.e., change "> adb shell pm path
com.android.webview" to "> adb shell pm path com.android.webview" and similarly
for the surrounding fence lines).
- Line 138: Fix the inverted version comparison in the docs note: change the
parenthetical from "(< 1.29)" to "(>= 1.29)" or reword to say "in versions 1.29
and later" so the sentence correctly communicates that the feature is not
possible on newer releases and that uninstalling updates to 1.24 is required;
update the text fragment "This is not possible with latest app version (< 1.29)"
to the corrected comparison or phrasing.
- Around line 108-109: The PowerShell pipeline passes the .assets array to
Invoke-WebRequest instead of the APK download URL; extract the
browser_download_url first and pass that string to Invoke-WebRequest. Update the
invocation that uses Invoke-RestMethod, .assets, Where-Object and Select-Object
-ExpandProperty browser_download_url so the result (or a local $url variable) is
what you feed into Invoke-WebRequest -Uri, then call Invoke-WebRequest -OutFile
ImmichFrame.apk with that URL.
Comment on lines
+82
to
+88
| > Webview provieds the following features that are not available without it: | ||
| > - SplitView | ||
| > - Some image formats (HEIC) | ||
| > - Video playback (experimenta) | ||
| > - Better text rendering | ||
| > - Show Tag, desc, and album info from Immich. | ||
| > - Configurable layout, (size, background and overlay colors, and blur effects, etc). |
There was a problem hiding this comment.
Fix typos in the WebView feature list.
The note block contains several typographical errors:
- Line 82: "provieds" should be "provides"
- Line 85: "experimenta" should be "experimental"
- Line 88: "etc)" should be "etc.)"
📝 Proposed fix for typos
-> Webview provieds the following features that are not available without it:
+> Webview provides the following features that are not available without it:
> - SplitView
> - Some image formats (HEIC)
-> - Video playback (experimenta)
+> - Video playback (experimental)
> - Better text rendering
> - Show Tag, desc, and album info from Immich.
-> - Configurable layout, (size, background and overlay colors, and blur effects, etc).
+> - Configurable layout, (size, background and overlay colors, and blur effects, etc.).🧰 Tools
🪛 LanguageTool
[grammar] ~82-~82: Ensure spelling is correct
Context: ...ameo Webview Update below. > > Webview provieds the following features that are not ava...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[style] ~88-~88: In American English, abbreviations like “etc.” require a period.
Context: ...d and overlay colors, and blur effects, etc). 1. You will need to install ADB o...
(ETC_PERIOD)
🤖 Prompt for AI Agents
In `@docs/docs/getting-started/apps.md` around lines 82 - 88, Correct
typographical errors in the Webview feature note: change the heading sentence
"Webview provieds the following features" to "Webview provides the following
features", change the list item "Video playback (experimenta)" to "Video
playback (experimental)", and change the trailing "etc)" to "etc.)" so the
feature list text is spelled and punctuated correctly.
Comment on lines
+108
to
+109
| Invoke-WebRequest -Uri (Invoke-RestMethod -Uri https://api.github.com/repos/immichFrame/ImmichFrame_Android/releases/latest).assets | Where-Object { $_.name -like "*.apk" } | Select-Object -First 1 -ExpandProperty browser_download_url | Invoke-WebRequest -OutFile ImmichFrame.apk | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n docs/docs/getting-started/apps.md | sed -n '105,115p'Repository: immichFrame/ImmichFrame
Length of output: 688
🏁 Script executed:
# Test the PowerShell command syntax
pwsh -NoProfile -Command '
# Original command from the review
$originalCmd = "Invoke-WebRequest -Uri (Invoke-RestMethod -Uri https://api.github.com/repos/immichFrame/ImmichFrame_Android/releases/latest).assets | Where-Object { `$_.name -like `"*.apk`" } | Select-Object -First 1 -ExpandProperty browser_download_url | Invoke-WebRequest -OutFile ImmichFrame.apk"
# Proposed fix
$fixedCmd = "Invoke-WebRequest -Uri ((Invoke-RestMethod -Uri https://api.github.com/repos/immichFrame/ImmichFrame_Android/releases/latest).assets | Where-Object { `$_.name -like `"*.apk`" } | Select-Object -First 1 -ExpandProperty browser_download_url) -OutFile ImmichFrame.apk"
Write-Host "Testing original command syntax..."
try {
[scriptblock]::Create($originalCmd) | Out-Null
Write-Host "Original command parses OK"
} catch {
Write-Host "Original command has syntax error: $_"
}
Write-Host "Testing fixed command syntax..."
try {
[scriptblock]::Create($fixedCmd) | Out-Null
Write-Host "Fixed command parses OK"
} catch {
Write-Host "Fixed command has syntax error: $_"
}
'Repository: immichFrame/ImmichFrame
Length of output: 184
🏁 Script executed:
pwsh -NoProfile -Command '
# Test original command logic - will it fail at runtime?
Write-Host "Testing original command execution logic..."
try {
# Simulate the command without actually hitting the API
$assets = @(
[PSCustomObject]@{name="ImmichFrame.apk"; browser_download_url="https://example.com/apk"},
[PSCustomObject]@{name="other.zip"; browser_download_url="https://example.com/zip"}
)
# This is what the original command tries to do:
Write-Host "Attempting: Invoke-WebRequest -Uri `$assets | Where-Object..."
$result = $assets | Where-Object { $_.name -like "*.apk" }
Write-Host "After pipeline: $($result | Get-Member -MemberType NoteProperty | Select-Object -ExpandProperty Name)"
} catch {
Write-Host "Error: $_"
}
Write-Host ""
Write-Host "The issue: the original command passes .assets array to -Uri instead of extracting the URL first"
Write-Host "The .assets array object cannot be converted to a valid URI"
'Repository: immichFrame/ImmichFrame
Length of output: 371
Fix the PowerShell command to extract the download URL before passing it to Invoke-WebRequest.
The command has a logic error: it passes the .assets array object directly to the -Uri parameter instead of extracting the URL string first. This will fail at runtime.
🔧 Corrected PowerShell command
Invoke-WebRequest -Uri ((Invoke-RestMethod -Uri https://api.github.com/repos/immichFrame/ImmichFrame_Android/releases/latest).assets | Where-Object { $_.name -like "*.apk" } | Select-Object -First 1 -ExpandProperty browser_download_url) -OutFile ImmichFrame.apkOr more readably:
$url = (Invoke-RestMethod -Uri https://api.github.com/repos/immichFrame/ImmichFrame_Android/releases/latest).assets | Where-Object { $_.name -like "*.apk" } | Select-Object -First 1 -ExpandProperty browser_download_url
Invoke-WebRequest -Uri $url -OutFile ImmichFrame.apk🤖 Prompt for AI Agents
In `@docs/docs/getting-started/apps.md` around lines 108 - 109, The PowerShell
pipeline passes the .assets array to Invoke-WebRequest instead of the APK
download URL; extract the browser_download_url first and pass that string to
Invoke-WebRequest. Update the invocation that uses Invoke-RestMethod, .assets,
Where-Object and Select-Object -ExpandProperty browser_download_url so the
result (or a local $url variable) is what you feed into Invoke-WebRequest -Uri,
then call Invoke-WebRequest -OutFile ImmichFrame.apk with that URL.
|
|
||
| On first reboot after install you will be asked to select default Launcher, select `ImmichFrame` and select "Always" | ||
|
|
||
|  |
There was a problem hiding this comment.
Add alt text to image for accessibility.
The image is missing alt text, which is important for screen readers and when images fail to load.
♿ Proposed fix to add alt text
-
+📝 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
|  | |
|  |
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 133-133: Images should have alternate text (alt text)
(MD045, no-alt-text)
🤖 Prompt for AI Agents
In `@docs/docs/getting-started/apps.md` at line 133, The markdown image tag
"" is missing alt text; update
that image reference to include a descriptive alt string (for example "Frameo
digital photo frame default app") so screen readers and broken-image contexts
have meaningful text, i.e., modify the markdown image tag to include the alt
text between the brackets.
docs/docs/getting-started/apps.md
Outdated
| 10. Then disable the Frameo app to set ImmichFrame as only Home app: | ||
|
|
||
| > [!NOTE] | ||
| > This is not possible with latest app version (< 1.29) so uninstalling updates (to 1.24) is required to get the overwriting of default home app to work. If you have already updated the Frameo app, you can uninstall updates by going to Android Settings-Apps-Frameo-Uninstall Updates. |
There was a problem hiding this comment.
Correct the version comparison logic.
The note states "not possible with latest app version (< 1.29)" which is contradictory. Based on the context (uninstalling updates to get back to 1.24), it appears the feature doesn't work on newer versions, but the comparison operator is reversed.
🔢 Proposed fix for version logic
-> This is not possible with latest app version (< 1.29) so uninstalling updates (to 1.24) is required to get the overwriting of default home app to work. If you have already updated the Frameo app, you can uninstall updates by going to Android Settings-Apps-Frameo-Uninstall Updates.
+> This is not possible with app version 1.29 and above, so uninstalling updates (to 1.24) is required to get the overwriting of default home app to work. If you have already updated the Frameo app, you can uninstall updates by going to Android Settings-Apps-Frameo-Uninstall Updates.Or alternatively:
-> This is not possible with latest app version (< 1.29) so uninstalling updates (to 1.24) is required to get the overwriting of default home app to work. If you have already updated the Frameo app, you can uninstall updates by going to Android Settings-Apps-Frameo-Uninstall Updates.
+> This is not possible with Frameo app versions >= 1.29, so uninstalling updates (to 1.24) is required to get the overwriting of default home app to work. If you have already updated the Frameo app, you can uninstall updates by going to Android Settings-Apps-Frameo-Uninstall Updates.📝 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
| > This is not possible with latest app version (< 1.29) so uninstalling updates (to 1.24) is required to get the overwriting of default home app to work. If you have already updated the Frameo app, you can uninstall updates by going to Android Settings-Apps-Frameo-Uninstall Updates. | |
| > This is not possible with app version 1.29 and above, so uninstalling updates (to 1.24) is required to get the overwriting of default home app to work. If you have already updated the Frameo app, you can uninstall updates by going to Android Settings-Apps-Frameo-Uninstall Updates. |
🤖 Prompt for AI Agents
In `@docs/docs/getting-started/apps.md` at line 138, Fix the inverted version
comparison in the docs note: change the parenthetical from "(< 1.29)" to "(>=
1.29)" or reword to say "in versions 1.29 and later" so the sentence correctly
communicates that the feature is not possible on newer releases and that
uninstalling updates to 1.24 is required; update the text fragment "This is not
possible with latest app version (< 1.29)" to the corrected comparison or
phrasing.
|
|
||
| 2. Push new apk to sdcard | ||
| > [!NOTE] | ||
| > Android does not have auto complete so you will save time if the new file is /sdcard/webview.apk, so you can just run `adb push /path/to/webview.apk /sdcard/webview.apk` and it will be named webview.apk on the device. |
There was a problem hiding this comment.
Use hyphen for compound adjective.
The compound adjective "auto complete" should be hyphenated as "auto-complete".
📝 Proposed fix
-> > Android does not have auto complete so you will save time if the new file is /sdcard/webview.apk, so you can just run `adb push /path/to/webview.apk /sdcard/webview.apk` and it will be named webview.apk on the device.
+> > Android does not have auto-complete so you will save time if the new file is /sdcard/webview.apk, so you can just run `adb push /path/to/webview.apk /sdcard/webview.apk` and it will be named webview.apk on the device.📝 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
| > Android does not have auto complete so you will save time if the new file is /sdcard/webview.apk, so you can just run `adb push /path/to/webview.apk /sdcard/webview.apk` and it will be named webview.apk on the device. | |
| > Android does not have auto-complete so you will save time if the new file is /sdcard/webview.apk, so you can just run `adb push /path/to/webview.apk /sdcard/webview.apk` and it will be named webview.apk on the device. |
🧰 Tools
🪛 LanguageTool
[grammar] ~197-~197: Use a hyphen to join words.
Context: ...[!NOTE] > Android does not have auto complete so you will save time if the ne...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
In `@docs/docs/getting-started/apps.md` at line 197, Replace the unhyphenated
compound adjective "auto complete" with the hyphenated form "auto-complete" in
the sentence containing "`adb push /path/to/webview.apk /sdcard/webview.apk`"
(the line that currently reads "Android does not have auto complete...") so the
documentation uses the correct compound adjective form.
| ``` | ||
| After reboot, you should see a "installing new app" notification, and then you can verify the new WebView version by going to Android `Settings-Apps`, top right corner triple dot menu `show system`, Android System WebView. | ||
|
|
||
|  |
There was a problem hiding this comment.
Add alt text to image for accessibility.
The image is missing alt text, which is important for screen readers and when images fail to load.
♿ Proposed fix to add alt text
-
+🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 223-223: Images should have alternate text (alt text)
(MD045, no-alt-text)
🤖 Prompt for AI Agents
In `@docs/docs/getting-started/apps.md` at line 223, The markdown image tag
currently has no alt text for Frameo_digital_photo_frames_webview.png; update
the tag  to include a concise,
descriptive alt string (for example: ) so screen readers and cases
where the image fails to load have meaningful text.
Comment on lines
+227
to
+229
| > ```shell | ||
| > adb shell pm path com.android.webview | ||
| > ``` |
There was a problem hiding this comment.
Remove extra spaces after blockquote symbol.
Line 227 has multiple spaces after the blockquote symbol (>), which is inconsistent with markdown formatting conventions.
📝 Proposed fix
> [!NOTE]
> Depending on the device or Android version, the location to the webview apk may be different for you. You can locate it with:
-> ```shell
+> ```shell
> adb shell pm path com.android.webview
> ```🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 227-227: Multiple spaces after blockquote symbol
(MD027, no-multiple-space-blockquote)
🤖 Prompt for AI Agents
In `@docs/docs/getting-started/apps.md` around lines 227 - 229, Fix the blockquote
spacing in the fenced code block: remove the extra spaces after the blockquote
symbol on the lines that currently read as quoted shell lines (the lines
wrapping "```shell", "adb shell pm path com.android.webview", and "```") so each
line uses a single ">" followed immediately by the content (i.e., change "> adb
shell pm path com.android.webview" to "> adb shell pm path com.android.webview"
and similarly for the surrounding fence lines).
Updated instructions for setting ImmichFrame as the default HOME app and clarified the note regarding Frameo app updates.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/docs/getting-started/apps.md`:
- Around line 129-131: The numbered step 9 has inconsistent indentation in the
markdown; update the block containing "Set as default HOME app:" and the
following indented instruction line ("On first reboot after install you will be
asked to select default Launcher, select `ImmichFrame` and select \"Always\"")
so its indentation matches the other numbered steps (use the same two-space or
four-space indent pattern as steps above) to ensure the markdown list renders
correctly.
Comment on lines
+129
to
+131
| 9. Set as default HOME app: | ||
|
|
||
| On first reboot after install you will be asked to select default Launcher, select `ImmichFrame` and select "Always" |
There was a problem hiding this comment.
Fix inconsistent list indentation for step 9.
Line 129 is indented differently from the other numbered steps, which can break markdown list rendering. Align the indentation to match the surrounding steps.
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 129-129: Inconsistent indentation for list items at the same level
Expected: 2; Actual: 1
(MD005, list-indent)
🤖 Prompt for AI Agents
In `@docs/docs/getting-started/apps.md` around lines 129 - 131, The numbered step
9 has inconsistent indentation in the markdown; update the block containing "Set
as default HOME app:" and the following indented instruction line ("On first
reboot after install you will be asked to select default Launcher, select
`ImmichFrame` and select \"Always\"") so its indentation matches the other
numbered steps (use the same two-space or four-space indent pattern as steps
above) to ensure the markdown list renders correctly.
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.
Major rewrite of the Frameo installation instructions
docs/docs/getting-started/apps.mdSummary by CodeRabbit