Preview/ember-template-lint#99
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (1)
Disabled knowledge base sources:
📝 WalkthroughWalkthroughAdds three new files: a CodeRabbit configuration ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bad.hbs`:
- Line 32: Remove the accidental "Collapse comment" text that was appended after
the <button> element containing "Load More Photos" in the bad.hbs template;
locate the <button>Load More Photos</button> instance and delete the trailing
"Collapse comment" artifact so only the intended button markup remains.
- Around line 6-7: The "Log In" and "Reset" button elements must declare
explicit types to avoid accidental form submissions—locate the <button> element
with text "Log In" and change it to include type="submit", and locate the
<button> element with text "Reset" and change it to include type="reset"; leave
the other buttons (e.g., "Add to Cart", "Buy Now" and those not inside a form)
unchanged.
- Around line 3-52: The template contains hardcoded UI strings and missing
attributes that break linting and accessibility: wrap every visible string in
the localization helper (e.g., change "Premium Widget", "Only $29.99 — Order now
and save 20%!", every <p>, <span>, <h1>/<h2>/<h3>, button labels like "Add to
Cart", "Buy Now", "Log In", "Reset", "Load More Photos", "Menu", "Submit",
"Cancel", and image captions with {{t "..."}}), add type="button" to all bare
<button> elements (buttons with text "Add to Cart", "Buy Now", "Log In",
"Reset", "Load More Photos", "Menu", "Submit", "Cancel"), add meaningful alt
attributes to all <img> tags (e.g., avatar.png, hero.jpg, icon.png, sunset.jpg,
beach.jpg, forest.jpg), add for="..." on <label> elements and corresponding
id="..." on their <input> elements (labels "Username" and "Password" -> link to
their inputs), and remove or properly wrap the orphaned text node "Collapse
comment" (delete it or include it inside a localized element). Ensure these
changes reference the existing template elements (headings h1/h2/h3, <p>,
<span>, <button>, <img>, <label>, <input>) so linters and accessibility checks
pass.
- Around line 10-13: The labels "Username" and "Password" are not associated
with their inputs; update the <label> and corresponding <input> elements so each
label has a for="..." that matches an id="..." on its input (e.g., label
for="username" with input id="username", and label for="password" with input
id="password"), ensuring the input types and placeholders remain unchanged and
adding unique ids if needed to avoid collisions.
🪄 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.yml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 986c9f6c-24d9-431d-ac06-6156276e2094
📒 Files selected for processing (3)
.coderabbit.yml.template-lintrc.jsonbad.hbs
| <h3>Premium Widget</h3> | ||
| <p>Only $29.99 — Order now and save 20%!</p> | ||
| <span>Free shipping on all orders</span> | ||
| <button>Add to Cart</button> | ||
| <button>Buy Now</button> | ||
| </article> | ||
| <form> | ||
| <label>Username</label> | ||
| <input type="text" placeholder="Enter username"> | ||
| <label>Password</label> | ||
| <input type="password"> | ||
| <button>Log In</button> | ||
| <button>Reset</button> | ||
| <p>Forgot your password?</p> | ||
| </form> | ||
| <section> | ||
| <h2>Photo Gallery</h2> | ||
| <div> | ||
| <img src="sunset.jpg"> | ||
| <p>A beautiful sunset over the mountains</p> | ||
| </div> | ||
| <div> | ||
| <img src="beach.jpg"> | ||
| <p>Sandy beach with crystal clear water</p> | ||
| </div> | ||
| <div> | ||
| <img src="forest.jpg"> | ||
| <p>Dense forest trail in autumn</p> | ||
| </div> | ||
| <button>Load More Photos</button>Collapse comment | ||
| </section> | ||
| <nav> | ||
| <h1>My Application</h1> | ||
| <ul> | ||
| <li>Home</li> | ||
| <li>About Us</li> | ||
| <li>Services</li> | ||
| <li>Contact</li> | ||
| </ul> | ||
| <button>Menu</button> | ||
| <img src="avatar.png"> | ||
| </nav> | ||
| <div> | ||
| <h1>Welcome to our site</h1> | ||
| <p>Click the button below</p> | ||
| <button>Submit</button> | ||
| <img src="hero.jpg"> | ||
| <span>Contact us</span> | ||
| <button>Cancel</button> | ||
| <img src="icon.png"> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="$(fd '^bad\.hbs$' | head -n1)"
echo "Inspecting: $file"
echo -e "\nButtons without explicit type:"
rg -nP '<button(?![^>]*\btype=)[^>]*>' "$file" || true
echo -e "\nImages without alt:"
rg -nP '<img(?![^>]*\balt=)[^>]*>' "$file" || true
echo -e "\nPotential bare strings between tags:"
rg -nP '>\s*[A-Za-z][^<{]*<' "$file" || true
echo -e "\nLabels without for=:"
rg -nP '<label(?![^>]*\bfor=)[^>]*>' "$file" || trueRepository: coderabbitai/coderabbit-pr-review
Length of output: 1593
Localize hardcoded UI copy to comply with the configured no-bare-strings rule.
All text content across lines 3–52 must be wrapped in a localization helper (e.g., {{t "..."}}) or the template will fail linting. Additionally:
- Buttons (lines 6, 7, 14, 15, 32, 42, 48, 51): Add
type="button"attribute - Images (lines 2, 21, 25, 29, 43, 49, 52): Add
altattribute for accessibility - Labels (lines 10, 12): Add
for=attribute to associate with input fields - Line 32: Remove the orphaned text node "Collapse comment" or wrap it properly
Proposed localization pattern
- <h3>Premium Widget</h3>
- <p>Only $29.99 — Order now and save 20%!</p>
- <button>Add to Cart</button>
+ <h3>{{t "product.premium_widget.title"}}</h3>
+ <p>{{t "product.premium_widget.promo_text"}}</p>
+ <button type="button">{{t "actions.add_to_cart"}}</button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bad.hbs` around lines 3 - 52, The template contains hardcoded UI strings and
missing attributes that break linting and accessibility: wrap every visible
string in the localization helper (e.g., change "Premium Widget", "Only $29.99 —
Order now and save 20%!", every <p>, <span>, <h1>/<h2>/<h3>, button labels like
"Add to Cart", "Buy Now", "Log In", "Reset", "Load More Photos", "Menu",
"Submit", "Cancel", and image captions with {{t "..."}}), add type="button" to
all bare <button> elements (buttons with text "Add to Cart", "Buy Now", "Log
In", "Reset", "Load More Photos", "Menu", "Submit", "Cancel"), add meaningful
alt attributes to all <img> tags (e.g., avatar.png, hero.jpg, icon.png,
sunset.jpg, beach.jpg, forest.jpg), add for="..." on <label> elements and
corresponding id="..." on their <input> elements (labels "Username" and
"Password" -> link to their inputs), and remove or properly wrap the orphaned
text node "Collapse comment" (delete it or include it inside a localized
element). Ensure these changes reference the existing template elements
(headings h1/h2/h3, <p>, <span>, <button>, <img>, <label>, <input>) so linters
and accessibility checks pass.
| <button>Add to Cart</button> | ||
| <button>Buy Now</button> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "bad.hbs" -o -name "*.hbs" | head -20Repository: coderabbitai/coderabbit-pr-review
Length of output: 87
🏁 Script executed:
cat -n bad.hbsRepository: coderabbitai/coderabbit-pr-review
Length of output: 1646
Buttons inside <form> must declare explicit types to prevent accidental form submissions.
Lines 14-15 lack type attributes:
<button>Log In</button>should betype="submit"<button>Reset</button>should betype="reset"
Buttons at lines 6-7, 32, 42, 48, and 51 are not inside form elements, so they don't pose a form submission risk.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bad.hbs` around lines 6 - 7, The "Log In" and "Reset" button elements must
declare explicit types to avoid accidental form submissions—locate the <button>
element with text "Log In" and change it to include type="submit", and locate
the <button> element with text "Reset" and change it to include type="reset";
leave the other buttons (e.g., "Add to Cart", "Buy Now" and those not inside a
form) unchanged.
| <label>Username</label> | ||
| <input type="text" placeholder="Enter username"> | ||
| <label>Password</label> | ||
| <input type="password"> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "bad.hbs" -type fRepository: coderabbitai/coderabbit-pr-review
Length of output: 87
🏁 Script executed:
cat -n bad.hbsRepository: coderabbitai/coderabbit-pr-review
Length of output: 1646
🏁 Script executed:
rg -t handlebars '{{t\s' --max-count=5Repository: coderabbitai/coderabbit-pr-review
Length of output: 158
🏁 Script executed:
rg -t handlebars '\{\{t ' --max-count=5Repository: coderabbitai/coderabbit-pr-review
Length of output: 112
🏁 Script executed:
rg '\{\{t ' -g '*.hbs' --max-count=10Repository: coderabbitai/coderabbit-pr-review
Length of output: 59
🏁 Script executed:
fd -e hbs -type fRepository: coderabbitai/coderabbit-pr-review
Length of output: 248
🏁 Script executed:
fd '\\.hbs$'Repository: coderabbitai/coderabbit-pr-review
Length of output: 59
🏁 Script executed:
git ls-files | head -20Repository: coderabbitai/coderabbit-pr-review
Length of output: 133
🏁 Script executed:
cat .template-lintrc.json && echo "---" && cat README.mdRepository: coderabbitai/coderabbit-pr-review
Length of output: 229
Associate labels with inputs for accessible form navigation.
Lines 10 and 12 labels are not bound to corresponding inputs. Add for/id pairs to establish the association.
Suggested fix
- <label>Username</label>
- <input type="text" placeholder="Enter username">
- <label>Password</label>
- <input type="password">
+ <label for="username">Username</label>
+ <input id="username" type="text" placeholder="Enter username">
+ <label for="password">Password</label>
+ <input id="password" type="password">📝 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.
| <label>Username</label> | |
| <input type="text" placeholder="Enter username"> | |
| <label>Password</label> | |
| <input type="password"> | |
| <label for="username">Username</label> | |
| <input id="username" type="text" placeholder="Enter username"> | |
| <label for="password">Password</label> | |
| <input id="password" type="password"> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bad.hbs` around lines 10 - 13, The labels "Username" and "Password" are not
associated with their inputs; update the <label> and corresponding <input>
elements so each label has a for="..." that matches an id="..." on its input
(e.g., label for="username" with input id="username", and label for="password"
with input id="password"), ensuring the input types and placeholders remain
unchanged and adding unique ids if needed to avoid collisions.
| <img src="forest.jpg"> | ||
| <p>Dense forest trail in autumn</p> | ||
| </div> | ||
| <button>Load More Photos</button>Collapse comment |
There was a problem hiding this comment.
Remove accidental text artifact on Line 32.
Collapse comment appears to be unintended UI content appended after the button.
🧹 Proposed fix
- <button>Load More Photos</button>Collapse comment
+ <button type="button">{{t "gallery.load_more"}}</button>📝 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.
| <button>Load More Photos</button>Collapse comment | |
| <button type="button">{{t "gallery.load_more"}}</button> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bad.hbs` at line 32, Remove the accidental "Collapse comment" text that was
appended after the <button> element containing "Load More Photos" in the bad.hbs
template; locate the <button>Load More Photos</button> instance and delete the
trailing "Collapse comment" artifact so only the intended button markup remains.
| inheritance: false | ||
| reviews: | ||
| profile: assertive | ||
| review_details: true No newline at end of file |
There was a problem hiding this comment.
Strong suggestion: explicitly wire emberTemplateLint so the preview actually exercises the tool path.
With inheritance: false and no reviews.tools section, this config never explicitly enables the ember-template-lint integration. Empirically, the 4 CodeRabbit inline comments on this PR (bad.hbs:7/:13/:32/:52) contain zero 🪛 ember-template-lint tool blocks and zero 🧰 Tools blocks — they read as generic HTML/accessibility observations from the LLM reviewer, not tool-backed output. By comparison, PR #97 (Preview/Stylelint) surfaces two 🪛 Stylelint (17.9.0) blocks, proving tool-backed output is identifiable when it actually runs.
The second commit (1d636b6 "show tooling in review detils") added review_details: true, which is a sensible step but doesn't add the tool entry the preview needs. Adding review_details: true alone does not turn on a tool that the config has not explicitly enabled.
The canonical tool key is emberTemplateLint (camelCase) per the CodeRabbit schema.v2.json reviews.tools properties — ember-template-lint (kebab-case) is not a recognized key. Note that PR #96 (Preview/EmberTemplate)'s current head ships the kebab-case form, which is also why that branch has zero 🪛 ember-template-lint tool blocks despite explicit wiring; PR #96 is a more explicit failed attempt, while this PR has even less wiring than that.
Suggested config:
inheritance: false
reviews:
profile: assertive
review_details: true
tools:
emberTemplateLint:
enabled: trueSeverity: strong suggestion (not a blocker — this is a demo/preview branch that doesn't merge).
| "rules": { | ||
| "no-bare-strings": "error", | ||
| "require-button-type": "error", | ||
| "require-valid-alt-text": "error" |
There was a problem hiding this comment.
Strong suggestion: add at least one Ember-specific rule + matching fixture so tool provenance is distinguishable.
The three enabled rules (no-bare-strings, require-button-type, require-valid-alt-text) overlap entirely with generic HTML/accessibility advice that any LLM reviewer would surface, so any review output on this branch can be fully explained by the LLM reading bad.hbs rather than by ember-template-lint actually running. Concrete evidence on this PR: CodeRabbit's bad.hbs:13 inline comment flags label-input association, which would map to require-input-label — but that rule is not in this config. That's an LLM observation layered on top of the configured rule set, not strict tool output.
To make tool provenance unambiguous, add at least one rule that an LLM is unlikely to flag from generic HTML knowledge alone, plus a matching fixture line in bad.hbs:
{
"rules": {
"no-bare-strings": "error",
"require-button-type": "error",
"require-valid-alt-text": "error",
"no-implicit-this": "error",
"deprecated-each-syntax": "error"
}
}Then add corresponding bait, e.g.:
<h1>Hello {{formatName}}!</h1>(triggersno-implicit-this— wants{{this.formatName}}){{#each items}}{{this}}{{/each}}(triggersdeprecated-each-syntax— wants{{#each items as |item|}})
Once one of those Ember-specific rule names appears in a CodeRabbit 🪛 ember-template-lint (X.Y.Z) block, the preview is demonstrably exercising the tool path rather than just the generic LLM reviewer.
Severity: strong suggestion (not a blocker — this is a demo/preview branch that doesn't merge).
Review SummaryVerdict: comment-only — no blockers. OverviewThis PR adds three files ( FindingsTwo strong suggestions are left as inline comments on the config files:
One additional softer suggestion — the auto-generated CodeRabbit release-notes summary in the PR body describes the fixture as if it were a real feature ("Added product showcase with pricing and purchase options", "Added login form with password recovery functionality"). For a fixture/preview PR, replacing or augmenting the body with an explicit fixture-intent disclosure would help future reviewers immediately see the demo intent. CodeRabbit's existing four inline comments on Status noteThis PR was closed at |
2 participants
Summary by CodeRabbit