InAppNotification Display gets corrupted when pushing messages with different methods in queue/stack #3391
Assignees
Labels
Milestone
Comments
michael-hawker
added
bug 🐛
|
Hello michael-hawker, thank you for opening an issue with us! I have automatically added a "needs triage" label to help get things started. Our team will analyze and investigate the issue, and escalate it to the relevant team if possible. Other community members may also look into the issue and provide feedback 🙌 |
|
Noticed while reviewing #3368, but not specifically related to that PR was a problem before, so filing this here for tracking. |
|
This issue has been marked as "needs attention 👋" due to no activity for 15 days. Please triage the issue so the fix can be established. |
9 tasks
ghost
pushed a commit
that referenced
this issue
Mar 4, 2021
<!-- 🚨 Please Do Not skip any instructions and information mentioned below as they are all required and essential to evaluate and test the PR. By fulfilling all the required information you will be able to reduce the volume of questions and most likely help merge the PR faster 🚨 --> <!-- 📝 It is preferred if you keep the "☑️ Allow edits by maintainers" checked in the Pull Request Template as it increases collaboration with the Toolkit maintainers by permitting commits to your PR branch (only) created from your fork. This can let us quickly make fixes for minor typos or forgotten StyleCop issues during review without needing to wait on you doing extra work. Let us help you help us! 🎉 --> ## Fixes #3391 <!-- Add the relevant issue number after the "#" mentioned above (for ex: Fixes #1234) which will automatically close the issue once the PR is merged. --> <!-- Add a brief overview here of the feature/bug & fix. --> In the InAppNotifications sample page, there is the option to display a notification with or without a custom DataTemplate. Mixing the usage of these buttons in the sample app with a StackMode other than "Replace", would cause cause the notifications to show double close buttons, or be out of alignment. ## PR Type What kind of change does this PR introduce? <!-- Please uncomment one or more that apply to this PR. --> - Bugfix <!-- - Feature --> <!-- - Code style update (formatting) --> <!-- - Refactoring (no functional changes, no api changes) --> <!-- - Build or CI related changes --> <!-- - Documentation content changes --> <!-- - Sample app changes --> <!-- - Other... Please describe: --> ## What is the current behavior? <!-- Please describe the current behavior that you are modifying, or link to a relevant issue. --> Currently, the sample page for InAppNotifications shows the bug when interacting with the "... (with/without DataTemplate)" buttons. This is due to some odd wiring in the code behind. Though 3 notification objects were defined in XAML, only two were actually used. The third is configured to show the custom data template; But when it was time to show another notification it hot swaps templates with the custom notification, and attempts to show the custom content. Unfortunately this causes a bunch of issues because the need to display the close button and what content to show gets out of sync when we start to close the stack of notifications. ## What is the new behavior? <!-- Describe how was this issue resolved or changed? --> In the update sample page, I've started using the third notification object. It maintains a separate stack because it is a separate notification, but so does the VS styled notification. The advise to developers going forward is to pick a model, not mix and match. Hot-swapping templates is not supported. To support the flow of the sample, I added an option to pass an argument to the Dismiss function. `InAppNotification.Dismiss(bool dismissAll = false)` Passing `true` will dismiss all of the notifications in the stack. I read that there was a comment in the sample code-behind saying, "Do not dismiss all in production", but I disagree. Sometimes you really need to dismiss them all, such as during major app navigations (sign in/out). If a whole stack of notifications becomes suddenly irrelevant we should be able to dismiss them all. It's useful in the sample as well, for when we switch between the three notifications. Lastly, in this process I uncovered a bug that causes the notification not to render when reusing the same DataTemplate. Here is the snippet from InAppNotification.cs ``` // From InAppNotifcation.UpdateContent(NotificationOptions notificationOptions) case DataTemplate dataTemplate: // Without this check, the dataTemplate will fail to render. // Why? Setting the ContentTemplate causes the control to re-evaluate it's Content value. // When we set the ContentTemplate to the same instance of itself, we aren't actually changing the value. // This means that the Content value won't be re-evaluated and stay null, causing the render to fail. if (_contentProvider.ContentTemplate != dataTemplate) { _contentProvider.ContentTemplate = dataTemplate; _contentProvider.Content = null; } break; ``` ## PR Checklist Please check if your PR fulfills the following requirements: - [ ] Tested code with current [supported SDKs](../readme.md#supported) - [ ] Pull Request has been submitted to the documentation repository [instructions](..\contributing.md#docs). Link: <!-- docs PR link --> - [x] Sample in sample app has been added / updated (for bug fixes / features) - [ ] Icon has been created (if new sample) following the [Thumbnail Style Guide and templates](https:/windows-toolkit/WindowsCommunityToolkit-design-assets) - [ ] New major technical changes in the toolkit have or will be added to the [Wiki](https:/windows-toolkit/WindowsCommunityToolkit/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) - [x] Header has been added to all new source files (run *build/UpdateHeaders.bat*) - [x] Contains **NO** breaking changes <!-- If this PR contains a breaking change, please describe the impact and migration path for existing applications below. Please note that breaking changes are likely to be rejected within minor release cycles or held until major versions. --> ## Other information Todo: - [ ] Document the new dismissAll flag.
ghost
locked as resolved and limited conversation to collaborators
This issue was closed.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Describe the bug
If you use different show methods with the InAppNotification when using the
StackModeofQueueBehindorStackInFrontthen the component display can get corrupted and show multiple close buttons or things out of place.Steps to Reproduce
Steps to reproduce the behavior:
StackModetoQueueBehindorStackInFrontExpected behavior
Message should show the same way it does if it was the only one sent.
Screenshots
Environment
Additional context
Add any other context about the problem here.
The text was updated successfully, but these errors were encountered: