[PR-Reviewer] Standardize PR review output format with collapsible sections

[kubaflo]

[PR-Reviewer] Standardize PR review output format with collapsible sections

Summary

This PR enhances the PR reviewer agent's output format to improve readability and consistency. Reviews now use a standardized collapsible format that surfaces key information upfront while keeping detailed analysis easily accessible.

Changes Made

1. Mandatory Review File Format

File: .github/instructions/pr-reviewer-agent/output-format.md

• All reviews must be saved as Review_Feedback_Issue_XXXXX.md files
• Implemented two-tier structure:
  • Always visible: Recommendation, required changes, and suggested improvements
  • Collapsible details: Full code review, testing results, security analysis, and metadata
• Added comprehensive template with all required sections
• Specified clear formatting requirements for consistency

2. Clarified PR Title Requirements

File: .github/instructions/pr-reviewer-agent/core-guidelines.md

• Made [PR-Reviewer] prefix mandatory for all agent-generated PRs
• Added explicit rule and purpose documentation
• Referenced new collapsible format requirement in review workflow

Benefits

• Improved Scannability: Reviewers can quickly see recommendations and action items without expanding full review
• Better Organization: All reviews follow consistent structure with clear section separation
• Full Context Available: Detailed analysis preserved in collapsible section for thorough review
• Clear Attribution: Mandatory [PR-Reviewer] prefix identifies agent-generated PRs at a glance

Example Output

The new format produces reviews like:

# Review Feedback: PR #XXXXX - [Title]

**Recommendation**: ✅ Approve with minor improvements

**Required changes**:
1. Fix platform attribute in test

<details>
<summary>📋 For full PR Review from agent, expand here</summary>
[Full detailed review with all sections]
</details>

Review Feedback: PR #32187 - Refactor SearchView color application logic

Recommendation: ⚠️ Request Changes - Minor documentation improvement needed

Required changes:

1. Update comment to use XML documentation format (/// <summary>) for the ApplyDecorColor method

Recommended changes:
None - code is otherwise excellent

---

📋 For full PR Review from agent, expand here

Summary

PR #32187 successfully fixes issue #25153 by refactoring SearchView color application logic. The PR extracts duplicate magnifier icon tinting code into a new ApplyDecorColor method and adds the missing underline tinting functionality. This ensures both the search icon and underline update correctly when the app theme changes from dark to light or vice versa.

Code Quality: Excellent refactoring that follows DRY principles and improves maintainability.
Bug Fix: Correctly addresses the root cause (missing search_plate background tinting).
Test Coverage: All 20 existing SearchBar visual test snapshots have been updated to reflect the fix.

---

Code Review

Root Cause Analysis

The Original Bug (Issue #25153):
When theme changes from dark to light (or vice versa), the SearchBar's underline and search icon don't update to match the new theme colors. The issue was tracked to UpdatePlaceholderColor and UpdateTextColor methods only tinting the magnifier icon but NOT tinting the underline (search_plate background).

Why This Fix Works:

1. When theme changes, Android calls UpdateTextColor or UpdatePlaceholderColor with the new theme color
2. These methods now call ApplyDecorColor(searchView, color)
3. ApplyDecorColor ensures BOTH decorative elements (icon + underline) are updated atomically
4. This guarantees consistency and fixes the theme change bug

Changes Made

Before: Duplicate code in two locations

// In UpdatePlaceholderColor and UpdateTextColor:
var searchMagIconImage = searchView.FindViewById<ImageView>(Resource.Id.search_mag_icon);
searchMagIconImage?.Drawable?.SetTint(color);
// Missing: underline tinting

After: Refactored into reusable method

// New ApplyDecorColor method:
static void ApplyDecorColor(SearchView searchView, Color color)
{
    var searchMagIconImage = searchView.FindViewById<ImageView>(Resource.Id.search_mag_icon);
    searchMagIconImage?.Drawable?.SetTint(color);

    var searchPlate = searchView.FindViewById(Resource.Id.search_plate);
    searchPlate?.Background?.SetTint(color);  // ← FIXES BUG
}

// Called from both UpdatePlaceholderColor and UpdateTextColor:
ApplyDecorColor(searchView, color);

Code Quality Assessment

✅ Excellent:

1. DRY Principle: Eliminates duplication of icon tinting code (was in 2 places)
2. Single Responsibility: ApplyDecorColor has one clear purpose
3. Maintainability: Future changes to decor coloring only need to be made in one place
4. Null Safety: Proper use of ?. operators
5. Naming: Method name clearly describes what it does (ApplyDecorColor)
6. Bug Fix: Addresses the root cause by adding missing underline tinting
7. Minimal Changes: Focused refactoring that doesn't introduce unnecessary modifications

⚠️ Minor Improvement Needed:

1. Documentation Comment Style: The method uses a single-line comment instead of XML documentation format
   • Current: // Tints the magnifier icon and the underline
   • Should be:

     /// <summary>
     /// Tints the magnifier icon and the underline.
     /// </summary>

   • Why: XML documentation provides better IDE integration and is consistent with C# documentation standards

Design Decisions

When ApplyDecorColor is called:

• UpdatePlaceholderColor: Only when resetting to default theme color (i.e., when PlaceholderColor is null)
• UpdateTextColor: Only when resetting to default theme color (i.e., when TextColor is null)

When ApplyDecorColor is NOT called:

• When user sets a custom PlaceholderColor - only hint text color changes, decorative elements stay with theme colors
• When user sets a custom TextColor - only text color changes, decorative elements stay with theme colors

This is intentional design: decorative elements (icon/underline) always follow theme colors unless explicitly overridden via SearchIconColor property (added in PR #26759).

Platform Coverage

✅ Android-specific fix (appropriate):

• Issue Theme issue on Search bar control in .NET MAUI #25153 was reported on Android only
• Changes are isolated to SearchViewExtensions.cs (Android platform code)
• No cross-platform concerns

---

Test Coverage Review

Existing Tests Updated

✅ 20 visual test snapshots updated:

• All snapshots in src/Controls/tests/TestCases.Android.Tests/snapshots/android/ have been updated
• These snapshots capture the visual appearance of SearchBar with various property configurations
• Updated snapshots now show correct underline coloring after theme changes

Test scenarios covered:

• PlaceholderColorShouldChange
• SearchBar_InitialState_VerifyVisualState
• SearchBar_SetCancelButtonAndTextColor_VerifyVisualState
• SearchBar_SetPlaceholderAndPlaceholderColor_VerifyVisualState
• SearchBar_SetPlaceholderColorAndTextColor_VerifyVisualState
• SearchbarColorsShouldUpdate (specifically tests theme switching)
• ... and 14 more SearchBar visual tests

Test Coverage Assessment

✅ Adequate: The PR correctly updates existing visual tests rather than adding new ones. The existing test suite already covers theme switching scenarios (e.g., SearchbarColorsShouldUpdate), and the updated snapshots demonstrate the fix is working correctly.

From PR review discussion: PR author confirmed "There are already lots of tests for the search bar theming" and provided visual proof that the underline now updates correctly after theme changes.

---

Testing

Manual Testing (Not Performed)

Decision: Given the comprehensive existing test coverage and clear code logic, manual testing in Sandbox app was deemed unnecessary for this review. The PR:

1. Has minimal, focused changes that are easy to verify through code inspection
2. Updates 20 existing visual test snapshots demonstrating the fix works
3. Has been verified by the PR author and issue reporter
4. CI/CD pipelines have been run multiple times (/azp run commands in comments)

Rationale: The code change is straightforward (adding 2 lines to tint search_plate background), and the updated test snapshots provide visual proof the fix works correctly.

---

Security Review

✅ No security concerns:

• No external input handling
• No credential storage or sensitive data
• Uses existing Android SDK APIs safely (FindViewById, SetTint)
• Proper null-safe navigation with ?. operators prevents null reference exceptions

---

Breaking Changes

✅ No breaking changes:

• Refactoring is internal to SearchViewExtensions class
• Public API surface unchanged
• Only behavioral change is bug fix (underline now updates with theme, which is expected behavior)

Impact: Positive - apps that rely on default theme behavior will now see correct underline coloring on theme changes without code modifications.

---

Documentation

✅ Adequate with one improvement:

• Code change is self-explanatory
• Method has an inline comment explaining purpose
• Improvement needed: Comment should use XML documentation format for better IDE integration

---

Issues to Address

Must Fix Before Merge

1. Documentation Comment Format:

   // Current:
   // Tints the magnifier icon and the underline
   static void ApplyDecorColor(SearchView searchView, Color color)
   
   // Should be:
   /// <summary>
   /// Tints the magnifier icon and the underline.
   /// </summary>
   static void ApplyDecorColor(SearchView searchView, Color color)

   Why: Consistency with C# documentation standards and better IDE support.

Should Fix (Recommended)

None

Optional Improvements

1. Consider exposing underline color customization: A PR review comment suggested adding a SearchPlateColor bindable property to allow users to customize the underline color independently from theme colors. This would be a nice-to-have enhancement for future consideration but is outside the scope of this bug fix.

---

Approval Checklist

• Code solves the stated problem correctly
• Minimal, focused changes
• No breaking changes
• Appropriate test coverage exists (20 snapshots updated)
• No security concerns
• Follows .NET MAUI conventions (except XML doc comment)
• No auto-generated files modified
• Platform-specific code properly isolated
• Null safety handled correctly
• XML documentation format (pending fix)

---

Additional Observations

Positive Aspects

1. Community Engagement: Issue was reported by a community member, noticed by another community member (@AlleSchonWeg), and fixed by a team member (@kubaflo) - excellent collaboration!

2. Incremental Improvement: This PR builds on previous work (PR Implementation of Customizable Search Button Color for SearchBar Across Platforms #26759 added SearchIconColor), showing good evolution of the API.

3. Clear Communication: PR author provided video demonstration and responded thoroughly to reviewer questions.

4. Proper Testing: CI/CD pipelines were run multiple times to ensure tests pass.

Future Enhancements (Out of Scope)

From PR discussion, potential future enhancements could include:

• Separate SearchPlateColor property for independent underline color control
• Documentation updates for SearchIconColor property (currently .NET 10 only)

---

Review Metadata

• Reviewer: @copilot (PR Review Agent)
• Review Date: 2025-11-20
• PR Number: Refactor SearchView color application logic #32187
• Issue Number: Theme issue on Search bar control in .NET MAUI #25153
• Platforms Tested: None (code review only - existing CI/CD tests verified)
• Test Approach: Code analysis + existing test snapshot review

[Copilot]

Pull Request Overview

This PR standardizes the output format for PR reviewer agent feedback by introducing a collapsible structure that improves readability and scannability. The changes establish mandatory file naming conventions, a two-tier review format (visible summary + collapsible details), and clarify PR title requirements for agent-generated PRs.

Key Changes:

• Introduced mandatory Review_Feedback_Issue_XXXXX.md file naming convention
• Implemented collapsible review format with visible recommendations and detailed analysis in expandable section
• Clarified that [PR-Reviewer] prefix is mandatory for all agent-generated PRs

Reviewed Changes

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

File	Description
.github/instructions/pr-reviewer-agent/output-format.md	Added comprehensive template with collapsible format, mandatory file naming, and detailed section structure for reviews
.github/instructions/pr-reviewer-agent/core-guidelines.md	Clarified PR title requirement as mandatory with explicit rule and formatting expectations

[Copilot]

The "Approval Checklist" section includes a placeholder line "[Add specific items relevant to this PR]" which could be confusing. This line should either be:

1. Removed (since the list above already contains standard items)
2. Changed to a comment/instruction like <!-- Add any PR-specific checklist items here -->
3. Clarified as "- [ ] [Add any additional PR-specific items here]"

As currently written, it's ambiguous whether this text should appear literally in the output or is meant as an instruction to the agent.

Suggested change

	[Add specific items relevant to this PR]
	<!-- Add any PR-specific checklist items here -->

[Copilot]

Minor inconsistency in section header formatting. In the example provided in the PR description, the recommendation in the visible section uses bold text without a section header (**Recommendation**: ✅ Approve), but the template at line 12 shows it as a proper markdown section header (## Recommendation).

Consider updating line 10-16 to show the expected output format more clearly:

# Review Feedback: PR #XXXXX - [PR Title]

**Recommendation**: [Choose one: ✅ Approve / ⚠️ Request Changes / 💬 Comment / ⏸️ Paused]

**Required changes** (if any):

This would match the example in the PR description and be more explicit about the expected format.

Suggested change

	## Recommendation
	✅ **Approve** - Ready to merge
	⚠️ **Request Changes** - Issues must be fixed
	💬 **Comment** - Feedback but not blocking
	⏸️ **Paused** - Cannot complete review (conflicts, environment issues, etc.)
	**Recommendation**: [Choose one: ✅ Approve / ⚠️ Request Changes / 💬 Comment / ⏸️ Paused]