Add copy under QR code to clarify where you will go #45654
Add copy under QR code to clarify where you will go #45654noahtalerman wants to merge 2 commits into
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
WalkthroughThis PR updates the enrollment OTA template ( Possibly related issues
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate 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: 2
🤖 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 `@frontend/templates/enroll-ota.html`:
- Around line 630-641: In setIosIpadContent, deviceType is assigned without
declaration which creates an implicit global; fix by declaring a block-scoped
variable (e.g., let or const) named deviceType at the start of the function
(inside setIosIpadContent) and then set it in the if/else branches before using
it in the querySelectorAll loop to avoid leaking to the global scope.
- Around line 339-343: Remove the empty src="" attributes from the image
elements (e.g., the <img> with class "profile-downloaded-img" and the other
template image at the same spot) so the img tags do not include an invalid empty
src; leave the class and alt attributes intact since JavaScript will populate
the src at runtime (i.e., delete src="" from those <img> elements in
enroll-ota.html).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f6154330-b3fa-45a8-aacb-703904133efd
📒 Files selected for processing (1)
frontend/templates/enroll-ota.html
| <img | ||
| class="profile-downloaded-img" | ||
| src="" | ||
| alt="select profile downloaded in settings" /> | ||
| </div> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if the file exists and inspect the relevant lines
head -370 frontend/templates/enroll-ota.html | tail -50 | cat -nRepository: fleetdm/fleet
Length of output: 1807
🏁 Script executed:
# Also check line 354 area
sed -n '350,360p' frontend/templates/enroll-ota.html | cat -nRepository: fleetdm/fleet
Length of output: 464
🏁 Script executed:
# Search for JavaScript that sets src on these image classes
rg -A 5 -B 5 "profile-downloaded-img|install-profile-img" frontend/templates/enroll-ota.htmlRepository: fleetdm/fleet
Length of output: 1670
🏁 Script executed:
# Check if there's JavaScript code that populates src attributes for these images
rg "\.src\s*=" frontend/templates/enroll-ota.html -A 2 -B 2Repository: fleetdm/fleet
Length of output: 359
Remove empty src attributes from template images.
At lines 341 and 354, src="" is invalid HTML and should be omitted since JavaScript immediately sets the actual image paths before rendering.
Proposed fix
<div class="profile-downloaded-container">
<img
class="profile-downloaded-img"
- src=""
alt="select profile downloaded in settings" />
</div>
@@
<div class="install-profile-container">
- <img class="install-profile-img" src="" alt="select install" />
+ <img class="install-profile-img" alt="select install" />
</div>🧰 Tools
🪛 HTMLHint (1.9.2)
[error] 341-341: The attribute [ src ] of the tag [ img ] must have a value.
(src-not-empty)
🤖 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 `@frontend/templates/enroll-ota.html` around lines 339 - 343, Remove the empty
src="" attributes from the image elements (e.g., the <img> with class
"profile-downloaded-img" and the other template image at the same spot) so the
img tags do not include an invalid empty src; leave the class and alt attributes
intact since JavaScript will populate the src at runtime (i.e., delete src=""
from those <img> elements in enroll-ota.html).
| const setIosIpadContent = (platform) => { | ||
| if (platform === "ios") { | ||
| deviceType = "iPhone"; | ||
| } else if (platform === "ipad") { | ||
| deviceType = "iPad"; | ||
| } | ||
|
|
||
| const setEnrollTokenUrl = (url) => { | ||
| document.querySelector(".enroll-link").setAttribute("href", url); | ||
| }; | ||
| document | ||
| .querySelectorAll('[data-attribute="dynamic-device-type"]') | ||
| .forEach((el) => { | ||
| el.textContent = deviceType; | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "enroll-ota.html" -type fRepository: fleetdm/fleet
Length of output: 94
🏁 Script executed:
wc -l frontend/templates/enroll-ota.htmlRepository: fleetdm/fleet
Length of output: 96
🏁 Script executed:
sed -n '620,650p' frontend/templates/enroll-ota.html | cat -nRepository: fleetdm/fleet
Length of output: 1314
🏁 Script executed:
rg "deviceType" frontend/templates/enroll-ota.html -nRepository: fleetdm/fleet
Length of output: 168
🏁 Script executed:
rg "deviceType" frontend/templates/enroll-ota.html -B 2 -A 2Repository: fleetdm/fleet
Length of output: 394
🏁 Script executed:
sed -n '1,50p' frontend/templates/enroll-ota.html | cat -nRepository: fleetdm/fleet
Length of output: 1597
🏁 Script executed:
sed -n '600,630p' frontend/templates/enroll-ota.html | cat -nRepository: fleetdm/fleet
Length of output: 1203
🏁 Script executed:
sed -n '550,650p' frontend/templates/enroll-ota.html | cat -nRepository: fleetdm/fleet
Length of output: 4219
🏁 Script executed:
rg -n "<script" frontend/templates/enroll-ota.html | head -5Repository: fleetdm/fleet
Length of output: 108
🏁 Script executed:
sed -n '479,510p' frontend/templates/enroll-ota.html | cat -nRepository: fleetdm/fleet
Length of output: 1416
🏁 Script executed:
rg "use strict" frontend/templates/enroll-ota.htmlRepository: fleetdm/fleet
Length of output: 39
🏁 Script executed:
sed -n '475,485p' frontend/templates/enroll-ota.html | cat -nRepository: fleetdm/fleet
Length of output: 584
Declare deviceType to avoid implicit global leakage.
At lines 632/634, deviceType is assigned without declaration, creating an implicit global variable. The script does not use strict mode, so this will leak to global scope. Declare it as block-scoped to prevent side effects.
Proposed fix
const setIosIpadContent = (platform) => {
- if (platform === "ios") {
- deviceType = "iPhone";
- } else if (platform === "ipad") {
- deviceType = "iPad";
- }
+ const deviceType = platform === "ios" ? "iPhone" : "iPad";
document
.querySelectorAll('[data-attribute="dynamic-device-type"]')
.forEach((el) => {
el.textContent = deviceType;
});🤖 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 `@frontend/templates/enroll-ota.html` around lines 630 - 641, In
setIosIpadContent, deviceType is assigned without declaration which creates an
implicit global; fix by declaring a block-scoped variable (e.g., let or const)
named deviceType at the start of the function (inside setIosIpadContent) and
then set it in the if/else branches before using it in the querySelectorAll loop
to avoid leaking to the global scope.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #45654 +/- ##
==========================================
- Coverage 66.75% 66.74% -0.01%
==========================================
Files 2744 2744
Lines 219316 219316
Branches 10794 10794
==========================================
- Hits 146394 146387 -7
- Misses 59696 59701 +5
- Partials 13226 13228 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
georgekarrv
left a comment
georgekarrv
left a comment
There was a problem hiding this comment.
This PR sure did mangle the whole page for just 'adding some text'
3 participants
For the following quick win:
Summary by CodeRabbit