NinjaOne WYSIWYG: ranked Top Files/Folders, 2-col layout, dark mode

[JonathanPitre]

Summary

• Per-drive Top Files and Top Folders as full-width ranked tables (path, size, modified) above Cleanup
• Cleanup + File Types side-by-side (col-xl-6); bar charts removed from per-drive layout
• NinjaOne dark mode: stat-desc for muted text; explicit dark text on info-card title/body
• New ConvertTo-NinjaOneHtml parameters: -MaxTopFiles, -MaxTopFolders, -ShowAllResults (default true), -FooterSuffix (integrator branding)
• Footer: left-aligned UltraTree v{version} with optional suffix; module 1.0.2

Motivation

NinjaOne integrators were post-processing ConvertTo-NinjaOneHtml HTML to inject ranked tables, reshape the three-column layout, and patch dark-mode text. Production Invoke-UltraTreeDiskAnalysis (1.3.0+) uses ranked tables instead of the originally proposed 8-item bar charts.

Closes #1
Closes #2

Test plan

• Extended ConvertTo-NinjaOneHtml.Tests.ps1 for ranked tables, two-column layout, ShowAllResults, footer, dark mode
• Invoke-Pester under Module/src/Tests — 133 passed, 0 failed
• Manual: Get-FolderSizes -DriveLetter C | ConvertTo-NinjaOneHtml in NinjaOne WYSIWYG (light + dark mode)

[JonathanPitre]

Updated PR: fixes Access Errors info-card contrast in NinjaOne dark mode (see issue #2 screenshot). New-HtmlInfoCard now sets explicit dark text on info-title/info-description because WYSIWYG info-card.* backgrounds stay light-tinted in dark mode.

[JonathanPitre]

Let me run some more tests before merging anything. I'll keep you posted.

[freezscholte]

Let me run some more tests before merging anything. I'll keep you posted.

@JonathanPitre did you run the tests? I have not yet had time to look at this also overlooked your PR if all okay and I will test too we can add this in for sure

[JonathanPitre]

@freezscholte Yes — tests are green. I expanded this PR beyond the original 8-item bar chart proposal: NinjaOne production reports now use full ranked Top Files / Top Folders tables, a two-column Cleanup + File Types row, optional All Results (-ShowAllResults), and integrator footer suffix (-FooterSuffix). Dark-mode fixes from the earlier commits remain.

Pester: Invoke-Pester -Path Module/src/Tests — 133 passed, 0 failed.

Issues #1 and #2 are still addressed by this PR; #1’s UI is ranked tables instead of bar charts after field testing. Ready for your review when you have time.

[JonathanPitre]

Follow-up: small PSScriptAnalyzer cleanup (OutputType on New-HtmlWrapper, New-HtmlRankedStack aligned with other New-Html* helpers). UltraTree PSScriptAnalyzerSettings.psd1: 0 Error/Warning findings; Invoke-Pester: 133 passed.

No further module changes needed for the NinjaOne integrator

[freezscholte]

Thanks for this — and for the two issues that led to it. The reproduction detail in #2 (screenshot included) made it easy to confirm, and I appreciate that the PR came with tests rather than needing them asked for.

I ran the branch locally: PSScriptAnalyzer with the project settings reports 0 findings, and Pester goes from 103 passing on main to 112 on this branch with no new failures (the 9 Format-ByteSize failures I see are a pre-existing culture issue on my macOS box, not yours). The layout change itself renders correctly — Top Files above Top Folders full-width, Cleanup and File Types side by side. I like the direction.

Four things I'd like to resolve before merging. Three are small; one needs a decision from me as much as from you.

1. The badge foreground colors go the wrong way (New-HtmlTag.ps1, New-HtmlWrapper.ps1). Forcing color: #fff puts white on both #f0ad4e and #4ECDC4 at about 1.9:1, at 0.75rem. The wrapper change from #333 to white is a straight regression — #333 on #f0ad4e was roughly 6.5:1. Inline colors are exactly right for WYSIWYG (the wrapper <style> gets stripped, so those badges previously rendered with no background at all); it's just the foreground that needs to be dark on the light tints. Details inline.

2. Top Files is capped upstream in a way that makes the new defaults unreachable. Get-FolderSizes sorts and truncates Items to -Top (default 40) globally — across all drives, files and folders mixed. Since folders almost always outweigh individual files, MaxTopFiles = 50 can't be reached at default settings, and under -AllDrives those 40 rows get split across drives, which can leave Top Files nearly empty. That's the exact symptom #1 describes, so I don't want to close it on a table that can go blank in the common case.

This one is mine to decide, not yours. Two options: keep separate top-N file and folder lists in Get-FolderSizes (correct, but changes public behavior for every caller), or leave the plumbing alone and pick defaults that don't over-promise plus document the -Top interaction. I'm leaning toward the second for now and the first as a follow-up. Happy to hear if you've hit this in production with Invoke-UltraTreeDiskAnalysis — you'd know the real-world numbers better than I do.

3. Please drop chore(html): add SupportsShouldProcess to HTML helper functions (e049a85). That's on me for not making the reasoning discoverable: PSScriptAnalyzerSettings.psd1 already excludes PSUseShouldProcessForStateChangingFunctions, with the comment "New-Html* functions only return strings". These helpers are pure formatters, so ShouldProcess doesn't have anything to gate. It also has one real effect — New-HtmlWrapper -WhatIf now returns $null instead of HTML. The commit is self-contained, so a revert should lift cleanly without touching the rest.

4. Hardcoded version in a test (ConvertTo-NinjaOneHtml.Tests.ps1:124, :186) — Should -Match 'UltraTree v1.0.2' will fail on the next version bump. Worth reading it from the manifest.

A few smaller things, none blocking:

• The BOM added to ~14 files plus the double-to-single quote sweep across the tests makes the diff a lot larger than the change. Not wrong, just harder to review — no need to undo it, but I'd avoid bundling it next time.
• New-HtmlBarChart is now unused in the module (only Monolith/UltraTree.ps1 still calls it) yet picked up a new CardStyle parameter. Either drop the parameter or drop the function.
• Monolith/UltraTree.ps1 didn't get the same treatment — it still emits TreeSize v, the hardcoded #666/#888, and the old bar chart, so the two distributions now diverge on both issues. Happy to take that on myself if you'd rather not.
• docs/images/ninjaone-dark-mode-access-errors.png is a JPEG with a .png extension and isn't referenced anywhere in the repo. I think it belongs in the issue description rather than the tree.
• Module/docs/CHANGELOG.md and the function docs don't mention the four new parameters yet.
• New-HtmlRankedStack and the New-HtmlFileTypeTable call site both -replace against generated markup. It works today, but it'll silently no-op if the card HTML ever changes. Not worth reworking now — just flagging it as a place that'll bite later.

Last thing: CI hasn't actually run on this yet — the workflow has been sitting at action_required on all four pushes because it's a fork PR. I'm approving the run now, so we should get a real Windows result rather than relying on either of our local runs.

One more note on scope: #1 asked for a Top Files bar chart and this replaces the charts with ranked tables. Your reasoning in the description is sound and I'd rather have the tables, so I'm fine with it — just calling out that I'm making that call knowingly.

If you'd rather I just push items 1, 3 and 4 onto your branch, say the word and I will — maintainerCanModify is on, so it's a two-minute job and I don't want to hand you a chore list for what's mostly my own conventions.

[freezscholte]

Moving these badges to inline styles is the right fix — in a WYSIWYG field the wrapper <style> block is stripped, so class="tag" alone rendered with no background at all. It's color: #fff that I'd change.

Rendered, this gives:

• Healthy — white on #4ECDC4 ≈ 1.9:1
• Warning — white on #f0ad4e ≈ 1.9:1
• Critical — white on #d9534f ≈ 3.9:1

At font-size: 0.75rem these count as normal text, so AA wants 4.5:1 and only Critical is close. The two light tints need dark text:

switch ($Type) {
    'expired'  { $bgColor = Get-ThemeColor -Severity 'Danger';  $fgColor = '#fff' }
    'disabled' { $bgColor = Get-ThemeColor -Severity 'Warning'; $fgColor = '#333' }
    default    { $bgColor = Get-ThemeColor -Severity 'Success'; $fgColor = '#333' }
}

$style = "$baseStyle background-color: $bgColor; color: $fgColor;"

#333 lands both around 6.5:1, and both tints stay light in NinjaOne dark mode — same reasoning you used for the info-card fix, which I think is correct.

[freezscholte]

This one is a straight regression: #333 on #f0ad4e was roughly 6.5:1, white on it is about 1.9:1. The standalone wrapper is the one context where the CSS does survive, so it was already readable here before the change.

Could this go back to #333, matching whatever we settle on inline in New-HtmlTag? Worth keeping the two in sync so the fragment and the exported file don't drift.

(For what it's worth, the base .tag rule above already had white on #4ECDC4 before this PR — that's my pre-existing problem, not something you introduced. I'll take it in a separate pass.)

[freezscholte]

This is the substantive one, and it's an upstream problem rather than anything wrong with this block.

Get-FolderSizes.ps1:232 does:

$allResults.Items = $allResults.Items | Sort-Object -Property SizeBytes -Descending | Select-Object -First $Top

$Top defaults to 40 and is applied globally — one list, all drives, files and folders mixed. So by the time ScanResults.Items reaches here there are at most 40 rows in total, and because directories almost always outweigh individual files, most of them will be folders. Filtering to -not $_.IsDirectory can leave Top Files with a handful of rows or none, which is the symptom #1 was opened about.

Under -AllDrives it gets worse, since those 40 rows are split across drives before this per-drive filter runs.

Two ways out, and I think the choice is mine:

• Have Get-FolderSizes keep separate top-N file and folder lists. Correct, but it changes public behavior for every caller, so it wants its own PR.
• Leave the plumbing alone, set defaults that are honest about the 40-row ceiling, and document the -Top interaction in the parameter help.

I'm inclined toward the second here and the first as a follow-up. Have you hit this with real scans in Invoke-UltraTreeDiskAnalysis, or are you passing a larger -Top in production? That'd tell us how bad it is in practice.

The re-sort in this block is harmless either way — Items arrives sorted already, so it's belt-and-braces rather than wrong.

[freezscholte]

Tied to the comment on ConvertTo-NinjaOneHtml.ps1:104 — with -Top defaulting to 40 across all drives, MaxTopFiles = 50 can never be reached and 50 + 25 = 75 rows per drive is unreachable by a wide margin. Whatever we land on above should probably set these too, so the config doesn't describe a shape the data can't produce.

[freezscholte]

This is the SupportsShouldProcess commit — flagging it here as representative rather than commenting on all ten helpers.

My fault for not making the reasoning findable. PSScriptAnalyzerSettings.psd1 carries:

'PSUseShouldProcessForStateChangingFunctions'  # New-Html* functions only return strings

ShouldProcess is for gating side effects, and these build a string and return it — there's no state to protect, so the guard can only ever suppress output. It does have one live effect: New-HtmlWrapper -WhatIf now returns $null where it used to return HTML.

git revert e049a85 should lift the whole thing cleanly. The [OutputType([string])] attributes and help additions from ad732cd are worth keeping — those are a genuine improvement.

[freezscholte]

'UltraTree v1.0.2' will fail the moment the version moves. Same on line 186. Reading it from the manifest keeps it green across bumps:

$version = (Import-PowerShellDataFile $PathToManifest).ModuleVersion
$html | Should -Match "UltraTree v$([regex]::Escape($version))"

Worth noting $script:Config.Version and ModuleVersion are two separate sources of truth that this PR bumps in lockstep by hand — that's a pre-existing wart of mine, and a test reading from the manifest would at least catch them drifting apart.

[JonathanPitre]

Thanks for the thorough review — addressing everything in commit 990450c on this branch.

Blocking items

1. Badge colors — New-HtmlTag now uses #333 on Warning/Healthy tints and #fff on Critical; New-HtmlWrapper warning/disabled tags back to #333. Tests updated.
2. Top Files / -Top — Keeping MaxTopFiles=50 intentionally (files are the primary cleanup signal in our field reports). Documented the mixed Items list and -Top interaction in Get-FolderSizes / ConvertTo-NinjaOneHtml help plus README and configuration docs. We already pass a larger -Top in production. Happy to follow up with separate file/folder top-N lists in Get-FolderSizes; leaving ConvertTo-NinjaOneHtml: show Top Files bar chart alongside Top Folders #1 open until then.
3. SupportsShouldProcess — Reverted manually (equivalent to dropping e049a85). New-Html* helpers are plain [CmdletBinding()] again.
4. Version in tests — Footer reads $MyInvocation.MyCommand.Module.Version; removed duplicate $script:Config.Version. Tests assert against the manifest.

Also in 990450c

• Stroustrup else brace fixes (CI FormattingCheck failure on run 32912998397)
• Dropped unused CardStyle from New-HtmlBarChart
• Deleted misnamed docs/images/ninjaone-dark-mode-access-errors.png — fresh screenshots attached here / on NinjaOne dark mode: hardcoded muted text unreadable in WYSIWYG reports #2
• CHANGELOG + function docs for the four new parameters

Deferred (as discussed)

• Monolith — happy for you to take that
• -replace on generated card markup — noted for later
• BOM/quote sweep — acknowledged, will avoid bundling next time

Separate PR: Node 20 deprecation → Actions v5 bump (not mixed into this HTML PR).

Declining the offer to push 1/3/4 directly — all landed in 990450c. Pester: 133 passed locally; FormattingCheck clean.

[JonathanPitre]

I need to investigate the MaxTopFiles=50 a bit more. It makes more sense to see more files than folders by default IMO to identify the large files that can be deleted.