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 for responsive offcanvas #859

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

SerratedSharp
Copy link

@SerratedSharp SerratedSharp commented Aug 27, 2024

Smoke tested manually by launching Demo.WebAssembly project locally, navigating to offcanvas examples, and testing this new example as well as other examples to ensure all existing dismiss behavior still works.

I don't know if there'd be edge cases where the additional data-bs-target="#@Id" could potentially be a breaking change. I could set it to only be rendered if EnableDefaultClass=false so it could be gracefully opted into and avoid breakig change, but conditional attributes add a bit of noise since you either need @if duplicating the line or @attributes against a dictionary. Let me know if you have a preference.

…ault "offcanvas" class following BS docs for responsive offcanvas.

- Includes recommended workaround for dismiss button where the default class is ommitted for this scenario per twbs/bootstrap#36962
- Added example to demos for this scenario.

Tested using Demo.WebASsembly project.
- Fixes typo in commnt
@gvreddy04
Copy link
Contributor

@SerratedSharp Thank you for the PR. I'll take care of this.

@gvreddy04 gvreddy04 added this to the 3.0.1 milestone Aug 27, 2024
@gvreddy04
Copy link
Contributor

@SerratedSharp Were you able to replicate this issue here: https://demos.blazorbootstrap.com/offcanvas#sizes?

If you were able to replicate the issue, please attach a screen recording.

Per my analysis, this is working as expected, but it was handled slightly differently in the past. We are applying different CSS classes when different sizes are applied.

@gvreddy04 gvreddy04 removed this from the 3.1.0 milestone Oct 21, 2024
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.

Responsive Offcanvas requires suppressing class="offcanvas"
2 participants