Skip to content

bug(designerv2): Replace icon and update text on mcp wizard#8876

Merged
anthony0t merged 1 commit intomainfrom
tonytang/ff
Mar 5, 2026
Merged

bug(designerv2): Replace icon and update text on mcp wizard#8876
anthony0t merged 1 commit intomainfrom
tonytang/ff

Conversation

@anthony0t
Copy link
Contributor

@anthony0t anthony0t commented Mar 3, 2026

Commit Type

  • feature - New functionality
  • fix - Bug fix
  • refactor - Code restructuring without behavior change
  • perf - Performance improvement
  • docs - Documentation update
  • test - Test-related changes
  • chore - Maintenance/tooling

Risk Level

  • Low - Minor changes, limited scope
  • Medium - Moderate changes, some user impact
  • High - Major changes, significant user/system impact

What & Why

Replace the icon and updated the text for mcp wizard

Impact of Change

  • Users:
    Visual changes
  • Developers:
  • System:

Test Plan

  • Unit tests added/updated
  • E2E tests added/updated
  • Manual testing completed
  • Tested in: local

Contributors

Screenshots/Videos

image

Copilot AI review requested due to automatic review settings March 3, 2026 23:19
@anthony0t anthony0t added the risk:low Low risk change with minimal impact label Mar 3, 2026
@github-actions
Copy link

github-actions bot commented Mar 3, 2026

🤖 AI PR Validation Report

PR Review Results

Thank you for your submission! Here's detailed feedback on your PR title and body compliance:

PR Title

  • Current: bug(designerv2): Replace icon and update text on mcp wizard
  • Issue: None blocking — the title follows conventional commit style and clearly indicates the change is in designerv2 and affects the MCP wizard.
  • Recommendation: Optional: make the title slightly more specific about the icon used and the lock behavior, e.g. bug(designerv2): show locked connection icon and message in MCP tool wizard — purely optional.

Commit Type

  • Properly selected (fix).
  • Note: Only one commit type is selected which is correct.

Risk Level

  • Assessment: Low selected in the PR body and there is a risk:low label on the PR. This matches the code diff which shows a small UI/localization change (2 files, ~33 additions/35 deletions). Advised risk: risk:low.

⚠️ What & Why

  • Current: Replace the icon and updated the text for mcp wizard
  • Issue: Short and present, but grammar/tense is slightly inconsistent and could be clearer about which icon and what text change was made.
  • Recommendation: Use a concise present-tense description with one sentence of context. Example: Replace the step indicator with a lock/check icon and add an informational message when the MCP connection is locked.

⚠️ Impact of Change

  • Issue: Users is populated with "Visual changes" but Developers and System are left blank. While this is a UI-only change, you should still state the developer- and system-facing impact (even if "None"), e.g.:
    • Users: Visual change: new icon and lock message shown when MCP connection is locked.
    • Developers: Minor UI logic: uses CheckmarkLockRegular icon and new localization key; no API changes.
    • System: No perf or architecture impact expected.
  • Recommendation: Fill those fields so reviewers immediately understand scope.

⚠️ Test Plan

  • Assessment: Manual testing completed and a local screenshot is included (good). No unit or E2E tests were added.
  • Issue: If only manual testing is selected, please briefly explain why unit/E2E tests were not added (e.g., purely visual change gated by localization and CSS, snapshot tests or storybook visual tests not needed). Preferable: add a small unit/snapshot test or E2E step if you have existing coverage for this component.
  • Recommendation:
    • If adding automated tests is not appropriate, add one sentence explaining why (e.g., "purely presentational change; covered by component snapshot tests elsewhere" or "no behavior change requiring logic tests").
    • Otherwise add a snapshot/unit test for the component showing the locked state rendering or a small E2E check that the message appears when connection is locked.

⚠️ Contributors

  • Current: Empty.
  • Assessment: Optional, but please remember to credit PM/Designer/Reviewer if they contributed. Add names/handles if applicable.

Screenshots/Videos

  • Assessment: Screenshot provided demonstrating the change — good for a visual PR.

Summary Table

Section Status Recommendation
Title Optional: make slightly more specific about lock icon/message
Commit Type No change needed
Risk Level Matches code diff; low risk
What & Why ⚠️ Improve tense and clarity; say which icon/message are changed
Impact of Change ⚠️ Fill Developer/System impact (even if "None")
Test Plan ⚠️ Explain why manual-only or add snapshot/unit test for locked state
Contributors ⚠️ Add credits if applicable
Screenshots/Videos Screenshot included — good

Summary: This PR passes the PR-body/title validation. The advised risk level is risk:low, which matches the PR label and the size/scope of the change. Please make small improvements to the PR body for clarity (What & Why), explicitly state Developer/System impact (even if none), and either add a brief justification for manual-only testing or include a snapshot/unit test that covers the new locked-state UI.

Please update the PR body with the recommended clarifications and resubmit if desired — thanks for the clear title, template usage, and screenshot!


Last updated: Thu, 05 Mar 2026 18:29:28 GMT

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses a visual bug in the MCP tool wizard within the designer v2 panel. It replaces the emoji lock (🔒) used in the step indicator with a proper Fluent UI icon (CheckmarkLockRegular), removes the previously shown parameters-step description text, and adds a dedicated info MessageBar to communicate when the connection is locked.

Changes:

  • Replaces the emoji lock indicator with the CheckmarkLockRegular Fluent UI icon for the "step 1 locked" state in the wizard step indicator
  • Removes the step2Description string (old "Configure the parameters for this MCP tool" text) and replaces it with a connectionLockedInfo MessageBar shown specifically when the connection is locked on the parameters step
  • Removes the borderTop styling from the tools management section (visual cleanup)

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
libs/designer-v2/src/lib/ui/panel/recommendation/browse/mcpToolWizard.tsx Replaces emoji lock icon with CheckmarkLockRegular, removes step 2 description, adds locked-connection info message bar, cleans up border style
Localize/lang/strings.json Adds dK7mXq (new locked connection info string), removes kAJqcb (old step 2 description string)

You can also share your feedback on Copilot code review. Take the survey.

@github-actions
Copy link

github-actions bot commented Mar 3, 2026

📊 Coverage Check

The following changed files need attention:

⚠️ libs/designer-v2/src/lib/ui/panel/recommendation/browse/mcpToolWizard.tsx - 57% covered (needs improvement)

Please add tests for the uncovered files before merging.

@Azure Azure deleted a comment from Copilot AI Mar 5, 2026
@Azure Azure deleted a comment from Copilot AI Mar 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-validated risk:low Low risk change with minimal impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants