Skip to content

Docs: Fix mangled AppbarButtons.razor.cs merge#11419

Merged
danielchalmers merged 1 commit intoMudBlazor:devfrom
danielchalmers:fix-mangled-appbarbuttons-merge
May 27, 2025
Merged

Docs: Fix mangled AppbarButtons.razor.cs merge#11419
danielchalmers merged 1 commit intoMudBlazor:devfrom
danielchalmers:fix-mangled-appbarbuttons-merge

Conversation

@danielchalmers
Copy link
Member

@danielchalmers danielchalmers commented May 27, 2025

Description

Broken in #11382

PR 11382 load notifications was put in oninit is why we can't run docs.server.

Huh, there are definitely changes I didn't make or intend to make. Must have been a bad copilot run I didn't revert properly. Will fix

How Has This Been Tested?

Type of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation (fix or improvement to the website or code docs)

Checklist

  • The PR is submitted to the correct branch (dev).
  • My code follows the code style of this project.
  • I've added relevant tests.

Copilot AI reviewed May 27, 2025
View reviewed changes
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 fixes a mangled merge in AppbarButtons.razor.cs by reworking the component’s initialization logic. Key changes include replacing OnInitializedAsync with OnAfterRenderAsync for notification loading, removing the LayoutService event subscription, and initializing the _messages dictionary to null instead of an empty dictionary.

Comments suppressed due to low confidence (1)

src/MudBlazor.Docs/Shared/AppbarButtons.razor.cs:59

  • The change to using OnAfterRenderAsync removed the subscription to LayoutService.MajorUpdateOccurred. Please confirm that this removal is intentional and that layout updates no longer need to trigger a UI state update.
protected override async Task OnInitializedAsync()

src/MudBlazor.Docs/Shared/AppbarButtons.razor.cs
public partial class AppbarButtons
{
private IDictionary<NotificationMessage, bool> _messages = new Dictionary<NotificationMessage, bool>();
private IDictionary<NotificationMessage, bool> _messages = null;
Copy link

Copilot AI May 27, 2025

Choose a reason for hiding this comment

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

Setting _messages to null may lead to a NullReferenceException if it is accessed before notifications are loaded. Consider initializing it to an empty dictionary or adding null checks where applicable.

Suggested change
private IDictionary<NotificationMessage, bool> _messages = null;
private IDictionary<NotificationMessage, bool> _messages = new Dictionary<NotificationMessage, bool>();

Copilot uses AI. Check for mistakes.
Copy link
Member Author

@danielchalmers danielchalmers May 27, 2025

Choose a reason for hiding this comment

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

Leaving it as it originally was

@github-actions github-actions bot added bug Unexpected behavior or functionality not working as intended PR: needs review labels May 27, 2025
@sonarqubecloud
Copy link

sonarqubecloud bot commented May 27, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

versile2
versile2 approved these changes May 27, 2025
View reviewed changes
Copy link
Contributor

@versile2 versile2 left a comment

Choose a reason for hiding this comment

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

Returned to original, verified works and fixes problem with Docs.Server

@danielchalmers danielchalmers changed the title Fix mangled AppbarButtons.razor.cs merge Docs: Fix mangled AppbarButtons.razor.cs merge May 27, 2025
@danielchalmers danielchalmers merged commit 1b2f81c into MudBlazor:dev May 27, 2025
7 checks passed
@danielchalmers danielchalmers deleted the fix-mangled-appbarbuttons-merge branch May 27, 2025 16:08
This was referenced Jul 22, 2025
Closed
Closed
Closed
This was referenced Aug 1, 2025
Closed
Open
Open
Closed
Merged
Open
Closed
This was referenced Aug 14, 2025
Closed
Closed
Closed
Closed
Closed
This was referenced Aug 25, 2025
Merged
Closed
Closed
This was referenced Oct 6, 2025
Open
Open
Open
@dependabot dependabot bot mentioned this pull request Oct 15, 2025
Closed
@dependabot dependabot bot mentioned this pull request Nov 3, 2025
Merged
This was referenced Nov 12, 2025
Closed
Closed
This was referenced Nov 25, 2025
Closed
Closed
Open
This was referenced Dec 12, 2025
Closed
Closed
Closed
Closed
Closed
Closed
@dependabot dependabot bot mentioned this pull request Dec 21, 2025
Closed
This was referenced Dec 29, 2025
Closed
Closed
This was referenced Jan 9, 2026
Closed
Closed
Closed
Merged
Closed
Closed
This was referenced Feb 10, 2026
Merged
Closed
Open
This was referenced Feb 23, 2026
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@versile2 versile2 versile2 approved these changes

Assignees

No one assigned

Labels

bug Unexpected behavior or functionality not working as intended

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@danielchalmers @versile2 @ScarletKuro