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

InfoBar StackedNotificationsBehavior #4399

Closed
wants to merge 7 commits into from

Conversation

vgromfeld
Copy link
Contributor

@vgromfeld vgromfeld commented Nov 27, 2021

Fixes #4194

Following the discussion we had on adding new features to InAppNotificationControls, I investigated the ability to replace the WCT InAppNotificationContol by WinUI's InfoBar adding what is missing to it.

PR Type

  • Feature

What is the current behavior?

Currently, there is 2 options to display in app notifications:

  • WCT InAppNotificationControl
  • WinUI InfoBar

InAppNotificationControl supports stacking whereas InfoBar provides a better layout but does not have any advanced features

What is the new behavior?

I'm proposing a new StackedNotificationBehavior to add InAppNotification's behavior to InfoBar.

This new behavior handles:

  • notifications stacking
  • auto dismiss of notifications
  • keep notification visible when hovered

Each notification can be triggered using a Notification object containing:

  • a title
  • a message
  • a severity
  • a duration (if needed)

The default control configuration will be used for all notifications.
This default configuration can be overriden per notification using NotificationWithOverrides. This allows a notification to alter any property from the InfoBar control.

image

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tested code with current supported SDKs
  • New component
    • Pull Request has been submitted to the documentation repository instructions. Link:
    • Added description of major feature to project description for NuGet package (4000 total character limit, so don't push entire description over that)
    • If control, added to Visual Studio Design project
  • Sample in sample app has been added / updated (for bug fixes / features)
  • New major technical changes in the toolkit have or will be added to the Wiki e.g. build changes, source generators, testing infrastructure, sample creation changes, etc...
  • Tests for the changes have been added (for bug fixes / features) (if applicable)
  • Header has been added to all new source files (run build/UpdateHeaders.bat)
  • Contains NO breaking changes

@ghost
Copy link

ghost commented Nov 27, 2021

Thanks vgromfeld for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌

@ghost ghost requested review from michael-hawker and azchohfi November 27, 2021 22:48
@ghost ghost added accessibility ♿ feature request 📬 A request for new changes to improve functionality improvements ✨ labels Nov 27, 2021
@michael-hawker michael-hawker added this to the 7.2/8.0? milestone Nov 29, 2021
Copy link
Contributor

@XAML-Knight XAML-Knight left a comment

Choose a reason for hiding this comment

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

Nice implementation. Not convinced we need a separate NotificationWithOverrides class, though.

@michael-hawker michael-hawker added the partner ⏹ Label for issues that are being depended on or blocking Toolkit partners. label Dec 2, 2021
@ghost
Copy link

ghost commented Dec 2, 2021

This PR has been marked as "needs attention 👋" and awaiting a response from the team.

@ghost ghost added the no-recent-activity 📉 Open Issues that require attention label Jan 1, 2022
@ghost
Copy link

ghost commented Jan 1, 2022

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 15 days. It will be closed if no further activity occurs within 30 days of this comment.

@ghost
Copy link

ghost commented Jan 20, 2022

This PR has been marked as "needs attention 👋" and awaiting a response from the team.

@ghost ghost removed the no-recent-activity 📉 Open Issues that require attention label Jan 20, 2022
@michael-hawker
Copy link
Member

Build failing due to style cop rules:

D:\a\1\s\Microsoft.Toolkit.Uwp.UI.Behaviors\InfoBar\Notification.cs(6,1): error SA1208: Using directive for 'System' should appear before directive for 'Microsoft.UI.Xaml.Controls' [D:\a\1\s\Microsoft.Toolkit.Uwp.UI.Behaviors\Microsoft.Toolkit.Uwp.UI.Behaviors.csproj]
D:\a\1\s\Microsoft.Toolkit.Uwp.UI.Behaviors\InfoBar\Notification.cs(7,1): error SA1210: Using directives should be ordered alphabetically by the namespaces. [D:\a\1\s\Microsoft.Toolkit.Uwp.UI.Behaviors\Microsoft.Toolkit.Uwp.UI.Behaviors.csproj]
D:\a\1\s\Microsoft.Toolkit.Uwp.UI.Behaviors\InfoBar\StackedNotificationsBehavior.cs(6,1): error SA1208: Using directive for 'System' should appear before directive for 'Microsoft.UI.Xaml.Controls' [D:\a\1\s\Microsoft.Toolkit.Uwp.UI.Behaviors\Microsoft.Toolkit.Uwp.UI.Behaviors.csproj]
D:\a\1\s\Microsoft.Toolkit.Uwp.UI.Behaviors\InfoBar\StackedNotificationsBehavior.cs(7,1): error SA1208: Using directive for 'System.Collections.Generic' should appear before directive for 'Microsoft.UI.Xaml.Controls' [D:\a\1\s\Microsoft.Toolkit.Uwp.UI.Behaviors\Microsoft.Toolkit.Uwp.UI.Behaviors.csproj]

@ghost ghost removed the needs attention 👋 label Feb 11, 2022
@michael-hawker
Copy link
Member

@vgromfeld we've moved this over to the new Windows Community Toolkit Labs. 🎉 CommunityToolkit/Labs-Windows#211 - tracking experiment here: CommunityToolkit/Labs-Windows#210

I've copied over most of your PR (waiting for some sample improvements to make it easier to implement more samples over the next couple of weeks). Also, a bit tripped up on a build issue with long-path I think.

Anyway, going to close out this PR over here for now, as we'll continue that work forward and then bring the component back to the main repo after we re-organize for 8.0. Labs will allow us to continue development easily and gather more feedback, make improvements, etc... in the meantime! Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility ♿ feature request 📬 A request for new changes to improve functionality improvements ✨ labs 🧪 Marks an issue/PR involved with Toolkit Labs partner ⏹ Label for issues that are being depended on or blocking Toolkit partners.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] Prevent InAppNotification from disappearing when hovered
3 participants