Skip to content

Windows managed local account: design feedback on the Users tab and error tooltip - #50675

Merged
getvictor merged 3 commits into
mainfrom
victor-43488-mla-designer-feedback
Aug 6, 2026
Merged

Windows managed local account: design feedback on the Users tab and error tooltip#50675
getvictor merged 3 commits into
mainfrom
victor-43488-mla-designer-feedback

Conversation

@getvictor

@getvictor getvictor commented Aug 6, 2026

Copy link
Copy Markdown
Member

Fixes from @melpike's feedback.

Keep the Windows tab visible when Windows MDM is off rather than hiding it, and disable Create hidden admin with a tooltip pointing at Windows MDM, matching how the macOS tab already behaves.

  • Extracted TurnOnMdmTooltipWrapper to reduce code duplication

Reword the password-policy error fleetd reports so it reads as sentences. The whole string is generated by Fleet, not by Windows, and it is now shown to admins verbatim in the Show managed account tooltip.

Left-align that tooltip.

Related issue: Resolves #43488

Checklist for submitter

Testing

  • Added/updated automated tests
  • QA'd all new/changed functionality manually

Summary by CodeRabbit

Summary by CodeRabbit

  • New Features

    • Windows account setup remains visible when Windows MDM is unavailable.
    • Added guidance for enabling Apple or Windows MDM when required.
    • Managed-account error tooltips now show detailed failure information when available.
  • Bug Fixes

    • Prevented unavailable managed-account options from being selected.
    • Improved Windows account error messages and password-policy wording.
    • Improved visual feedback for disabled actions in dropdown menus.

…rror tooltip

Keep the Windows tab visible when Windows MDM is off rather than hiding it, and
disable Create hidden admin with a tooltip pointing at Windows MDM, matching how
the macOS tab already behaves. Hiding the tab left an admin with no way to
discover the setting or learn what to turn on to reach it.

Reword the password-policy error fleetd reports so it reads as sentences. The
whole string is generated by Fleet, not by Windows, and it is now shown to
admins verbatim in the Show managed account tooltip, so it is written for that
surface rather than in the usual lowercase Go error style.

Left-align that tooltip. Dropdown option tooltips are centered by default, which
reads poorly for several sentences of prose. Scoped to this one tooltip: the
centering rule is shared by every dropdown option tooltip in Fleet, so changing
it globally is a much larger visual change than this warrants.
@getvictor
getvictor requested a lite review from Copilot August 6, 2026 13:59
@getvictor

Copy link
Copy Markdown
Member Author

@coderabbitai full review

@getvictor

Copy link
Copy Markdown
Member Author

/agentic_review

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.

@codecov

codecov Bot commented Aug 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.34%. Comparing base (658ebbf) to head (3279f43).
⚠️ Report is 10 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #50675      +/-   ##
==========================================
+ Coverage   68.31%   68.34%   +0.03%     
==========================================
  Files        3953     3955       +2     
  Lines      252735   253395     +660     
  Branches    13372    13522     +150     
==========================================
+ Hits       172659   173190     +531     
- Misses      64637    64710      +73     
- Partials    15439    15495      +56     
Flag Coverage Δ
backend 69.59% <ø> (+0.03%) ⬆️
frontend 61.66% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

The Windows tab and account panel now render regardless of Windows MDM availability. The managed-account checkbox is disabled when Windows MDM is unavailable or GitOps mode is enabled. Apple and Windows MDM tooltip behavior now uses a shared wrapper. Host error tooltips display detailed managed-account errors when available. Windows managed-account operation and password-policy error messages were revised.

Possibly related PRs

  • fleetdm/fleet#49863: Adds Windows managed local account configuration and related API and GitOps support.
  • fleetdm/fleet#50088: Implements the Windows managed-account functionality refined by this UI and error-handling change.
  • fleetdm/fleet#50241: Introduces the Windows managed-account UI updated in this change.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the Windows managed local account UI feedback and error tooltip changes.
Description check ✅ Passed The description states the purpose, related issue, implementation details, and completed automated and manual testing items.
Linked Issues check ✅ Passed The changes support issue #43488 by updating Windows Users-page behavior, preserving macOS behavior, and exposing managed-account errors to admins.
Out of Scope Changes check ✅ Passed The UI, tooltip, styling, and fleetd error-message changes are directly related to the stated Windows managed local account objectives.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch victor-43488-mla-designer-feedback

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the Windows managed local account UX to be discoverable even when Windows MDM is off, and improves how managed-account failure details are presented to admins.

Changes:

  • Always render the Windows tab in the Users card and disable “Create hidden admin” with a tooltip linking to Windows MDM when not configured.
  • Reword the Windows password-policy failure string produced by orbit so it reads as admin-facing sentences.
  • Left-align the managed-account failure tooltip text in the Host actions dropdown (scoped to that one tooltip).

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
orbit/pkg/managedaccount/managedaccount_windows.go Adjusts Windows managed-account error strings surfaced to the server/UI.
frontend/pages/ManageControlsPage/SetupExperience/cards/Users/components/UsersForm/UsersForm.tsx Always shows the Windows tab/panel and wires Windows MDM configured state into the section.
frontend/pages/ManageControlsPage/SetupExperience/cards/Users/components/UsersForm/UsersForm.tests.tsx Updates tests to reflect Windows tab always present + checkbox disabled/enabled behavior.
frontend/pages/ManageControlsPage/SetupExperience/cards/Users/components/UsersForm/components/WindowsAccountSection/WindowsAccountSection.tsx Adds Windows MDM gating tooltip and disables managed-account checkbox when not configured.
frontend/pages/hosts/details/HostDetailsPage/HostActionsDropdown/helpers.tsx Wraps reported managed-account failure detail to allow custom alignment styling.
frontend/pages/hosts/details/HostDetailsPage/HostActionsDropdown/_styles.scss Adds left-alignment styling for the managed-account failure tooltip content.
Suppressed comments (2)

orbit/pkg/managedaccount/managedaccount_windows.go:171

  • The new password-policy message no longer mentions checking the minimum password length, even though NERR_PasswordTooShort is one of the handled return codes. Adding it back keeps the guidance actionable for the most common cause while preserving the sentence-style formatting.
		return fmt.Errorf(
			"%s. This device's password policy rejected the generated %d-character password. "+
				"Check any custom password filter on the host.",
			op, passwordLen)

orbit/pkg/managedaccount/managedaccount_windows.go:158

  • Similar to the reset path, "Creating …" becomes a standalone first sentence in the password-policy error. Using a failure phrasing makes the surfaced message read more naturally.
	if ret != 0 {
		return accountError(fmt.Sprintf("Creating %s", username), ret, len(password))
	}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread orbit/pkg/managedaccount/managedaccount_windows.go
@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Aug 6, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. GitOps tooltip masked 🐞 Bug ≡ Correctness
Description
In WindowsAccountSection, the new outer TooltipWrapper is only disabled when Windows MDM is
configured, so when GitOps mode is enabled and Windows MDM is off the UI can show
competing/misleading tooltip guidance for why the checkbox is disabled. This can send admins to
enable Windows MDM when the real blocker is GitOps mode/YAML, slowing remediation.
Code

frontend/pages/ManageControlsPage/SetupExperience/cards/Users/components/UsersForm/components/WindowsAccountSection/WindowsAccountSection.tsx[R56-59]

+          }
+          disableTooltip={isWindowsMdmEnabledAndConfigured}
+          underline={false}
        position="left"
-          tipOffset={8}
-          isInputField
-          renderChildren={(gitopsEnabled) => (
-            <ManagedAccountCheckbox
-              disabled={!!gitopsEnabled}
-              value={enableManagedLocalAccount}
-              onChange={onEnableManagedLocalAccountChange}
-            />
-          )}
-        />
Evidence
The Windows MDM tooltip wrapper is enabled whenever Windows MDM is off, but the checkbox is also
disabled by GitOps mode via GitOpsModeTooltipWrapper; because TooltipWrapper anchors a tooltip to
the wrapped element whenever disableTooltip is false, the Windows MDM tooltip remains active even in
GitOps mode and can compete with/mislead relative to the GitOps tooltip.

frontend/pages/ManageControlsPage/SetupExperience/cards/Users/components/UsersForm/components/WindowsAccountSection/WindowsAccountSection.tsx[42-74]
frontend/components/GitOpsModeTooltipWrapper/GitOpsModeTooltipWrapper.tsx[62-108]
frontend/components/TooltipWrapper/TooltipWrapper.tsx[115-148]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`WindowsAccountSection` wraps `GitOpsModeTooltipWrapper` with an outer `TooltipWrapper` that is enabled whenever Windows MDM is not configured (`disableTooltip` only considers `isWindowsMdmEnabledAndConfigured`). In GitOps mode, the checkbox is also disabled by `GitOpsModeTooltipWrapper`, and the GitOps tooltip is the important remediation path; the Windows MDM tooltip should not “win” in that state.
### Issue Context
- The outer tooltip is enabled/disabled solely by `isWindowsMdmEnabledAndConfigured`.
- GitOps mode state is only available inside `GitOpsModeTooltipWrapper` via its `renderChildren` callback.
### Fix Focus Areas
- frontend/pages/ManageControlsPage/SetupExperience/cards/Users/components/UsersForm/components/WindowsAccountSection/WindowsAccountSection.tsx[42-74]
Suggested approach:
- Move the `TooltipWrapper` *inside* the `GitOpsModeTooltipWrapper.renderChildren` callback so it can be disabled when `gitopsEnabled` is true (or render combined tip content when both conditions apply).
- Ensure only one tooltip is active for the checkbox hover target at a time (GitOps should take precedence when enabled).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Password hint less actionable 🐞 Bug ⚙ Maintainability
Description
In Windows managed-account provisioning, accountError no longer mentions checking the host’s
minimum password length when a password-policy rejection occurs, even though this Windows error code
is used for length policy violations. This reduces the primary remediation guidance shown verbatim
to admins in the UI.
Code

orbit/pkg/managedaccount/managedaccount_windows.go[R169-171]

+			"%s. This device's password policy rejected the generated %d-character password. "+
+				"Check any custom password filter on the host.",
  	op, passwordLen)
Evidence
The constant comment documents that this Windows return code is used for password policy failures
beyond custom filters (including length), but the updated message only tells admins to check custom
password filters, removing the previously present minimum-length remediation hint.

orbit/pkg/managedaccount/managedaccount_windows.go[31-37]
orbit/pkg/managedaccount/managedaccount_windows.go[162-174]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The updated `accountError` message for `NERR_PasswordTooShort` / `ERROR_PASSWORD_RESTRICTION` removes the explicit instruction to check the host’s minimum password length, leaving only a custom password filter hint.
### Issue Context
The code comment explains Windows uses this return code for a variety of password-policy rejections (including minimum length). The error string is surfaced to admins verbatim (per PR description), so it should preserve the most common remediation steps.
### Fix Focus Areas
- orbit/pkg/managedaccount/managedaccount_windows.go[31-37]
- orbit/pkg/managedaccount/managedaccount_windows.go[166-173]
Suggested change:
- Reword the message as sentences (as intended by this PR) but include both remediation hints, e.g.:
- `"%s. This device's password policy rejected the generated %d-character password. Check the minimum password length and any custom password filter on the host."`

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

Comment thread orbit/pkg/managedaccount/managedaccount_windows.go
Comment on lines +35 to +39
// Drop the stale highlight for as long as a disabled option is hovered.
.actions-dropdown-select__menu-list:has(.actions-dropdown-select__option--is-disabled:hover)
.actions-dropdown-select__option--is-focused {
background-color: transparent;
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This another bug fix.

@getvictor
getvictor marked this pull request as ready for review August 6, 2026 15:11
@getvictor
getvictor requested review from a team as code owners August 6, 2026 15:11
@getvictor
getvictor requested a review from juan-fdz-hawa August 6, 2026 15:11

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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/components/ActionsDropdown/_styles.scss`:
- Around line 35-39: Update the `.actions-dropdown-select__option--is-focused`
background override in the dropdown styles so it applies only during pointer
interaction, not when keyboard focus is active. Reuse the existing
`isKeyboardFocus` state or pointer-mode class from `ActionsDropdown.tsx`, while
preserving the stale-highlight removal for hovered disabled options.
🪄 Autofix

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: 72b5c514-29aa-46b3-8df5-bac06b124dca

📥 Commits

Reviewing files that changed from the base of the PR and between 50a26a5 and 3279f43.

📒 Files selected for processing (9)
  • frontend/components/ActionsDropdown/_styles.scss
  • frontend/pages/ManageControlsPage/SetupExperience/cards/Users/components/UsersForm/UsersForm.tests.tsx
  • frontend/pages/ManageControlsPage/SetupExperience/cards/Users/components/UsersForm/components/EndUserAuthSection/EndUserAuthSection.tsx
  • frontend/pages/ManageControlsPage/SetupExperience/cards/Users/components/UsersForm/components/LocalAccountSection/LocalAccountSection.tsx
  • frontend/pages/ManageControlsPage/SetupExperience/cards/Users/components/UsersForm/components/TurnOnMdmTooltipWrapper/TurnOnMdmTooltipWrapper.tsx
  • frontend/pages/ManageControlsPage/SetupExperience/cards/Users/components/UsersForm/components/TurnOnMdmTooltipWrapper/index.ts
  • frontend/pages/ManageControlsPage/SetupExperience/cards/Users/components/UsersForm/components/WindowsAccountSection/WindowsAccountSection.tsx
  • frontend/pages/hosts/details/HostDetailsPage/HostActionsDropdown/_styles.scss
  • frontend/pages/hosts/details/HostDetailsPage/HostActionsDropdown/helpers.tsx
💤 Files with no reviewable changes (1)
  • frontend/pages/ManageControlsPage/SetupExperience/cards/Users/components/UsersForm/UsersForm.tests.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
  • frontend/pages/hosts/details/HostDetailsPage/HostActionsDropdown/_styles.scss
  • frontend/pages/ManageControlsPage/SetupExperience/cards/Users/components/UsersForm/components/WindowsAccountSection/WindowsAccountSection.tsx
  • frontend/pages/hosts/details/HostDetailsPage/HostActionsDropdown/helpers.tsx

Comment on lines +35 to +39
// Drop the stale highlight for as long as a disabled option is hovered.
.actions-dropdown-select__menu-list:has(.actions-dropdown-select__option--is-disabled:hover)
.actions-dropdown-select__option--is-focused {
background-color: transparent;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Preserve the keyboard focus indicator.

Lines 36-38 clear .actions-dropdown-select__option--is-focused whenever a disabled option is hovered. This also occurs during keyboard navigation if the pointer remains over a disabled option. The focused option then has no visible state.

Scope this override to pointer interaction. Reuse the existing isKeyboardFocus behavior in ActionsDropdown.tsx, or apply a pointer-mode class before clearing the background.

🤖 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/components/ActionsDropdown/_styles.scss` around lines 35 - 39,
Update the `.actions-dropdown-select__option--is-focused` background override in
the dropdown styles so it applies only during pointer interaction, not when
keyboard focus is active. Reuse the existing `isKeyboardFocus` state or
pointer-mode class from `ActionsDropdown.tsx`, while preserving the
stale-highlight removal for hovered disabled options.

@getvictor
getvictor merged commit 49bf85c into main Aug 6, 2026
56 of 58 checks passed
@getvictor
getvictor deleted the victor-43488-mla-designer-feedback branch August 6, 2026 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Windows: Create a local admin account

3 participants