Website: add /capex-savings page - #50272
Open
eashaw wants to merge 6 commits into
Open
Conversation
fleet-release
previously approved these changes
Jul 30, 2026
Contributor
There was a problem hiding this comment.
Warning
- Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.
Pull request overview
Adds a new marketing landing page at /capex-savings, wiring it into Sails routing/policies and the website layout so it can render and run its page-specific frontend code.
Changes:
- Added new controller + route + policy entry for
GET /capex-savings(view-capex-savings). - Added a new EJS page template with a capex savings calculator UI.
- Included the new page’s JS bundle in the global website layout.
Reviewed changes
Copilot reviewed 5 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| website/views/pages/capex-savings.ejs | New landing page markup including calculator UI and CTA sections. |
| website/views/layouts/layout.ejs | Loads the new /js/pages/capex-savings.page.js bundle. |
| website/config/routes.js | Adds the /capex-savings route with meta title/description locals. |
| website/config/policies.js | Enables the view-capex-savings action via policies config. |
| website/api/controllers/view-capex-savings.js | New Sails controller to render the capex savings view. |
| website/assets/styles/importer.less | (Content excluded) Likely wires new page styles into the build. |
| website/assets/js/pages/capex-savings.page.js | (Content excluded) Page-specific JS for the calculator / page behavior. |
| website/assets/styles/pages/capex-savings.less | (Content excluded) Page-specific styling for /capex-savings. |
Files excluded by content exclusion policy (3)
- website/assets/js/pages/capex-savings.page.js
- website/assets/styles/importer.less
- website/assets/styles/pages/capex-savings.less
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+10
to
+11
| <img alt="a small green line" class="d-none d-sm-block" purpose="callout-line" src="/images/capex-cta-callout-line-90x42@2x.png"> | ||
| <img alt="a small green line" class="d-block d-sm-none" purpose="callout-line" src="/images/capex-cta-callout-line-sm-13x41@2x.png"> |
| <div purpose="page-section"> | ||
| <div purpose="feature-with-image"> | ||
| <div purpose="feature-image"> | ||
| <img alt="A table showing device memory presure" src="/images/dex-mem-pressure-800x600@2x.png"> |
fleet-release
approved these changes
Jul 30, 2026
Contributor
There was a problem hiding this comment.
Warning
- Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.
Pull request overview
Copilot reviewed 5 out of 15 changed files in this pull request and generated no new comments.
Files excluded by content exclusion policy (3)
- website/assets/js/pages/capex-savings.page.js
- website/assets/styles/importer.less
- website/assets/styles/pages/capex-savings.less
Comments suppressed due to low confidence (5)
website/views/pages/capex-savings.ejs:11
- These callout-line images appear decorative. To avoid noisy screen reader output, use empty alt text (and optionally aria-hidden).
<img alt="a small green line" class="d-none d-sm-block" purpose="callout-line" src="/images/capex-cta-callout-line-90x42@2x.png">
<img alt="a small green line" class="d-block d-sm-none" purpose="callout-line" src="/images/capex-cta-callout-line-sm-13x41@2x.png">
website/views/pages/capex-savings.ejs:32
- Alt text has a spelling error: "presure" → "pressure".
<img alt="A table showing device memory presure" src="/images/dex-mem-pressure-800x600@2x.png">
website/views/pages/capex-savings.ejs:95
- This range input doesn’t have an accessible name (no associated or aria-label). Add an aria-label (or wire up a proper ).
<input type="range" min="2" max="5" step="1" v-model.number="currentCycleYears">
website/views/pages/capex-savings.ejs:105
- The deviceCount inputs have mismatched constraints (range 100–50,000 vs number 1–1,000,000). This can produce out-of-range values that the slider can’t represent. Align the min/max/step (and give the range an accessible name).
<input type="range" min="100" max="50000" step="100" v-model.number="deviceCount">
<input type="number" min="1" max="1000000" step="100" v-model.number="deviceCount" aria-label="Devices in your fleet, exact number">
website/views/pages/capex-savings.ejs:86
- This range input doesn’t have an accessible name (no associated or aria-label). Add an aria-label (or wire up a proper ).
This issue also appears on line 95 of the same file.
<input type="range" min="1000" max="6000" step="100" v-model.number="costPerDevice">
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughAdds a public Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
website/assets/styles/pages/capex-savings.less (1)
15-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate
pselector blocks in the same scope.Two separate
p { ... }rules exist directly under#capex-savings(L15-18 and L32-37). The second silently overridesfont-size/line-height/colorset by.body-l()in the first, relying on cascade order rather than being merged into one block — easy to break on future edits.♻️ Proposed consolidation
p { - .body-l(); - margin-bottom: 0px; + .body-l(); + font-size: 16px; + line-height: `@text-lineheight`; + color: `@core-fleet-black-75`; + margin-bottom: 0px; } ... - p { - font-size: 16px; - line-height: `@text-lineheight`; - color: `@core-fleet-black-75`; - margin-bottom: 0px; - }Also applies to: 32-37
🤖 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 `@website/assets/styles/pages/capex-savings.less` around lines 15 - 18, Merge the duplicate p selector blocks under `#capex-savings` into one rule, preserving the .body-l(), margin-bottom, and declarations currently defined in both blocks. Remove the separate second p block so typography and color are defined together without cascade-order dependence.
🤖 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 `@website/assets/styles/pages/capex-savings.less`:
- Around line 154-184: Update the bottom CTA selectors in the capex savings
stylesheet to match the actual markup: target the bare cloud-city element, style
the CTA’s h1 and p instead of h2 and h4, and target the link with
purpose="cta-button" instead of purpose="contact-button". Preserve the intended
bottom CTA sizing, spacing, typography, and button padding/minimum width.
- Around line 6-9: Import the stylesheet that defines the heading mixins before
the h1 rule in capex-savings.less, matching the existing page stylesheet
pattern. Ensure .page-heading(), .section-heading(), and related mixin
references resolve during compilation without changing the rule styles.
---
Nitpick comments:
In `@website/assets/styles/pages/capex-savings.less`:
- Around line 15-18: Merge the duplicate p selector blocks under `#capex-savings`
into one rule, preserving the .body-l(), margin-bottom, and declarations
currently defined in both blocks. Remove the separate second p block so
typography and color are defined together without cascade-order dependence.
🪄 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 Plus
Run ID: b6acf0ef-bab6-4d3b-87ab-ead90094626d
⛔ Files ignored due to path filters (7)
website/assets/images/capex-cta-callout-272x61@2x.pngis excluded by!**/*.pngwebsite/assets/images/capex-cta-callout-line-90x42@2x.pngis excluded by!**/*.pngwebsite/assets/images/capex-cta-callout-line-sm-13x41@2x.pngis excluded by!**/*.pngwebsite/assets/images/dex-health-800x600@2x.pngis excluded by!**/*.pngwebsite/assets/images/dex-mem-pressure-800x600@2x.pngis excluded by!**/*.pngwebsite/assets/images/dex-score-breakdown-800x450@2x.pngis excluded by!**/*.pngwebsite/assets/images/dex-score-device-800x450@2x.pngis excluded by!**/*.png
📒 Files selected for processing (8)
website/api/controllers/view-capex-savings.jswebsite/assets/js/pages/capex-savings.page.jswebsite/assets/styles/importer.lesswebsite/assets/styles/pages/capex-savings.lesswebsite/config/policies.jswebsite/config/routes.jswebsite/views/layouts/layout.ejswebsite/views/pages/capex-savings.ejs
Comment on lines
+6
to
+9
| h1 { | ||
| .page-heading(); | ||
| margin-bottom: 0px; | ||
| } |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate .page-heading mixin definition and check stylelint's LESS syntax config.
rg -n 'page-heading' --type=less
fd -e less stylelint --iglob '*stylelint*'
cat .stylelintrc* 2>/dev/nullRepository: fleetdm/fleet
Length of output: 151
🏁 Script executed:
#!/bin/bash
set -u
echo "== repo status =="
git status --short
echo "== locate capex-savings less file =="
fd -a 'capex-savings\.less$' . || true
echo "== locate page-heading references =="
rg -n 'page-heading|section-heading|page-container|cta-button|feature-with-image|checklist' . || true
echo "== locate stylelint/config/package files =="
git ls-files | rg -n '(^|/)(package\.json|package-lock\.json|yarn\.lock|pnpm-lock\.yaml|stylelint\.|\.stylelintrc|\.stylelintignore)$|node_modules' || true
for f in $(git ls-files | rg '(^|/)(package\.json|package-lock\.json|yarn\.lock|pnpm-lock\.yaml|\.?stylelint.*)$|node_modules' | head -100); do
echo "--- $f"
case "$f" in
package.json) sed -n '1,220p' "$f" ;;
package-lock.json|yarn.lock|pnpm-lock.yaml) rg -n 'stylelint|postcss|less|less-loader|stylelint-order' "$f" || true ;;
.?stylelint*) sed -n '1,240p' "$f" ;;
node_modules) : ;; # skip dirs without cat
esac
done
echo "== inspect target file nearby =="
target="$(git ls-files | rg 'website/assets/styles/pages/capex-savings\.less$' | head -1 || true)"
if [ -n "$target" ]; then
wc -l "$target"
sed -n '1,140p' "$target" | cat -n
fiRepository: fleetdm/fleet
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -u
echo "== imports/style include at capex-savings.less top and matching imports elsewhere =="
sed -n '1,40p' website/assets/styles/pages/capex-savings.less | cat -n
echo "--- imports in security-and-control.less ---"
sed -n '1,8p' website/assets/styles/pages/security-and-control.less | cat -n
echo "--- imports in software-management.less ---"
sed -n '1,8p' website/assets/styles/pages/software-management.less | cat -n
echo "--- imports in homepage.less ---"
sed -n '1,8p' website/assets/styles/pages/homepage.less | cat -n
echo "== stylelint references in config/package files (narrowed) =="
for f in package.json package-lock.json yarn.lock pnpm-lock.yaml .stylelintrc.json .stylelintrc.js .stylelintignore; do
if [ -f "$f" ]; then
echo "--- $f"
sed -n '1,220p' "$f" | rg -n "stylelint|stylelint-config|postcss|less|plugins|extends|rules|ignoreFiles|cssnano|declaration-block-no-redundant-longhand-properties|value-keyword-case|function-calc-no-unspaced-operator|unit-no-unknown|at-rule-no-unknown|selector-pseudo-class-no-unknown|block-no-empty|color-no-invalid-hex|x11-nam" -C 2 || true
fi
done
echo "== LESS mixin definitions that are likely referenced =="
rg -n '^(\\.page-heading|\\.section-heading|\\.page-container|\\.cta-button|\\.feature-with-image|\\.checklist)\\(' website/assets/styles website/views website/generators 2>/dev/null | head -80 || true
echo "== target block full =="
sed -n '1,45p' website/assets/styles/pages/capex-savings.less | cat -nRepository: fleetdm/fleet
Length of output: 3713
Resolve the Unknown word .page-heading before relying on this stylesheet.
The file references .page-heading()/.section-heading()/etc. without importing the mixin file that defines them, unlike the matching page stylesheet patterns. This makes the LESS/Sass build fail before the rest of the page styles can be validated or compiled.
🧰 Tools
🪛 Stylelint (17.14.1)
[error] 7-7: Unknown word .page-heading (CssSyntaxError)
(CssSyntaxError)
🤖 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 `@website/assets/styles/pages/capex-savings.less` around lines 6 - 9, Import
the stylesheet that defines the heading mixins before the h1 rule in
capex-savings.less, matching the existing page stylesheet pattern. Ensure
.page-heading(), .section-heading(), and related mixin references resolve during
compilation without changing the rule styles.
Source: Linters/SAST tools
Comment on lines
+154
to
+184
| [purpose='bottom-cloud-city-banner'] { | ||
| background: linear-gradient(180deg, #E9F4F4 0%, #FFFFFF 100%); | ||
| img { | ||
| width: 100%; | ||
| } | ||
| } | ||
|
|
||
| [purpose='bottom-cta'] { | ||
| max-width: 796px; | ||
| gap: 32px; | ||
| display: flex; | ||
| flex-direction: column; | ||
| margin-right: auto; | ||
| margin-left: auto; | ||
| text-align: center; | ||
| h2 { | ||
| font-size: 48px; | ||
| margin-bottom: 0px; | ||
| } | ||
| h4 { | ||
| font-size: 16px; | ||
| } | ||
| [purpose='button-row'] { | ||
| margin-top: 24px; | ||
| [purpose='contact-button'] { | ||
| padding: 16px 32px; | ||
| min-width: 140px; | ||
| text-decoration: none; | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Dead CSS: selectors don't match the actual markup in capex-savings.ejs.
Cross-checking against website/views/pages/capex-savings.ejs:
[purpose='bottom-cloud-city-banner'](L154-159) has no matching element — the page just renders a bare<cloud-city>(ejs L128-129).[purpose='bottom-cta'] h2(L169-172) andh4(L173-175) never match — the bottom CTA markup uses<h1>See what your fleet data says</h1>and a<p>(ejs L120-121), so they fall back to the generic pageh1/pstyles instead of the intended 48px heading / 16px body override.[purpose='contact-button'](L178-182) never matches — the actual CTA link usespurpose="cta-button"(ejs L123), so this padding/min-width override is dead and the button instead only gets the generic hero-sized.cta-button()styling (140×36, L65-70).
The intended distinct styling for the bottom CTA section currently isn't applied.
🐛 Proposed fix (align selectors to actual markup)
[purpose='bottom-cta'] {
...
- h2 {
+ h1 {
font-size: 48px;
margin-bottom: 0px;
}
- h4 {
+ p {
font-size: 16px;
}
[purpose='button-row'] {
margin-top: 24px;
- [purpose='contact-button'] {
+ [purpose='cta-button'] {
padding: 16px 32px;
min-width: 140px;
text-decoration: none;
}
}
}🤖 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 `@website/assets/styles/pages/capex-savings.less` around lines 154 - 184,
Update the bottom CTA selectors in the capex savings stylesheet to match the
actual markup: target the bare cloud-city element, style the CTA’s h1 and p
instead of h2 and h4, and target the link with purpose="cta-button" instead of
purpose="contact-button". Preserve the intended bottom CTA sizing, spacing,
typography, and button padding/minimum width.
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
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.
Changes:
Summary by CodeRabbit
/capex-savings.