Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: FluentMessageBar onclick bug and not using Link?.Target #1462

Merged
merged 1 commit into from
Feb 6, 2024

Conversation

benniedejong
Copy link
Contributor

In the FluentMessageBar some @OnClick event bindings were broken since version 4.4.0, I replaced them with the new OnClick. If a Target was specified in the Link of a notification, it was not used in the FluentMessageBar and always fell back to _blank.

Pull Request

📖 Description

  1. Fix: When opening the FluentMessageBar, it would create the unhandledexception "Unhandled exception rendering component: Unable to set property 'onclick' on object of type 'Microsoft.FluentUI.AspNetCore.Components.FluentAnchor'. The error was: Arg_InvalidCastException", this has been fixed by replacing the @OnClick with OnClick

  2. If the Link in the Message would contain a target, this target was not used (hardcoded to _blank). Now it user the target in the link or _blank in case no target has been specified.

👩‍💻 Reviewer Notes

Add messages to the FluentMessageBar and open it.

📑 Test Plan

✅ Checklist

General

  • I have added tests for my changes.
  • ✅ I have tested my changes.
  • I have updated the project documentation to reflect my changes.
  • I have read the CONTRIBUTING documentation and followed the standards for this project.

Component-specific

  • I have added a new component
  • I have added Unit Tests for my new compontent
  • ✅ I have modified an existing component
  • I have validate Unit Tests for an existing component

⏭ Next Steps

In the FluentMessageBar some @OnClick event bindings were broken since version 4.4.0, I replaced them with the new OnClick. If a Target was specified in the Link of a notification, it was not used in the FluentMessageBar and always fell back to _blank.
@benniedejong
Copy link
Contributor Author

@microsoft-github-policy-service agree

@microsoft-github-policy-service agree

@dvoituron
Copy link
Collaborator

Could you also solve the same <FluentAnchor @onclick="" problem for other components?

  • NotificationCenterPanel.razor
  • FluentToast.razor

@benniedejong
Copy link
Contributor Author

Could you also solve the same <FluentAnchor @onclick="" problem for other components?

  • NotificationCenterPanel.razor
  • FluentToast.razor

Will do FluentToast.razor tonight, NotificationCenterPanel.razor is already in my commit.

@vnbaaij
Copy link
Collaborator

vnbaaij commented Feb 6, 2024

The fix for FluentToast is already taken care of in an upcoming PR from me. And NotificationCenterPanel as well, so that one will be double fixed 😉

@vnbaaij vnbaaij enabled auto-merge (squash) February 6, 2024 14:38
@vnbaaij vnbaaij disabled auto-merge February 6, 2024 15:30
@vnbaaij vnbaaij merged commit 55d58bc into microsoft:dev Feb 6, 2024
4 checks passed
vnbaaij pushed a commit that referenced this pull request Feb 6, 2024
In the FluentMessageBar some @OnClick event bindings were broken since version 4.4.0, I replaced them with the new OnClick. If a Target was specified in the Link of a notification, it was not used in the FluentMessageBar and always fell back to _blank.
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.

None yet

3 participants