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

feat: allow the ability to hide the dismissal button for the FluentMessageBar. #1495

Merged
merged 4 commits into from
Feb 11, 2024
Merged

feat: allow the ability to hide the dismissal button for the FluentMessageBar. #1495

merged 4 commits into from
Feb 11, 2024

Conversation

StevenRasmussen
Copy link
Contributor

Pull Request

📖 Description

Adds the AllowDismiss property to the FluentMessageBar component. Sometimes it would be beneficial to have a message bar displayed that is not dismissible by the user. This adds the property and sets the default to true so that existing instances do not break. I added another FluentMessageBar instance under the MessageBarSimple example to demonstrate the behavior and for documentation.

✅ 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

Sorry, something went wrong.

Verified

This commit was signed with the committer’s verified signature.
crazy-max CrazyMax
…ssageBar.
@vnbaaij
Copy link
Collaborator

vnbaaij commented Feb 11, 2024

Hi Steven, Thanks for this.
Before we can merge, please make sure the existing Unit Tests validate (pretty sure it will fial now with AllowDissmiss=false) and add some new Unit Tests to test this new behavior

@vnbaaij vnbaaij requested review from dvoituron and vnbaaij February 11, 2024 10:07
@dvoituron
Copy link
Collaborator

dvoituron commented Feb 11, 2024

I find this improvement interesting.
Could you include a screenshot of this new "design" in your PR, to check if the UI rules are respected?
Is the space originally taken up by this Dismiss Icon is available for the rest of the content?

@StevenRasmussen
Copy link
Contributor Author

Here is a screenshot of the demo app:
image

It appears to honor the UI rules and allows the content to fill up the entire space now. I'll take a look at the unit tests to see what needs to be updated there. Thanks for the consideration.

…eBar component.
@StevenRasmussen
Copy link
Contributor Author

I added another unit test for this new property. The existing unit test was still succeeding (as it should) since this property does not affect existing instances with the default set to true. Please let me know if you need anything else to complete this PR.

@dvoituron dvoituron merged commit 03c2b0f into microsoft:dev Feb 11, 2024
4 checks passed
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