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

Responsive Offcanvas requires suppressing class="offcanvas" #849

Open
SerratedSharp opened this issue Aug 24, 2024 · 2 comments · May be fixed by #859
Open

Responsive Offcanvas requires suppressing class="offcanvas" #849

SerratedSharp opened this issue Aug 24, 2024 · 2 comments · May be fixed by #859

Comments

@SerratedSharp
Copy link

SerratedSharp commented Aug 24, 2024

Propose to add an EnableDefaultClass parameter property that allows the default class to be suppressed as needed.

Describe the bug
Responsive offcanvas offcanvas-lg should not have class="offcanvas" per this example: https://getbootstrap.com/docs/5.3/components/offcanvas/#responsive

To Reproduce
<Offcanvas class="offcanvas-lg"> produces class="offcanvas offcanvas-lg" which breaks the responsiveness(showing content on page above breakpoint) and also makes the offcanvas transparent(when page width is above breakpoint and offcanvas is shown).

Screenshots
How the bug looks when larger than breakpoint and offcanvas shown:
image

Proposed Solution
Verified fix by inheriting from Offcanvas and overriding ClassNames to allow the default class to be disabled:

  [Parameter]
  public bool EnableDefaultClass { get; set; } = true;

  protected override string? ClassNames =>
      BuildClassNames(Class,
          (BootstrapClass.Offcanvas, EnableDefaultClass),
          (Placement.ToOffcanvasPlacementClass(), true),
          (Size.ToOffcanvasSizeClass(), true));

The only remaining problem is this line is hardcoded to expect a class "offcanvas" and so it breaks the dismiss button when class is suppressed. data-bs-dismiss="offcanvas" would need to be adjusted.

@if (ShowCloseButton)
{
    <button type="button" class="btn-close" data-bs-dismiss="offcanvas" aria-label="Close"></button>
}

Usage:
<OffcanvasEx @ref="offcanvas" EnableDefaultClass="false" class="offcanvas-lg">

Versions (please complete the following information):

  • .NET Version: [e.g. .NET 6, .NET 7, .NET 8]
  • BlazorBootstrap: 3.0.0 preview.3
  • Blazor WebAssembly
  • Blazor Interactive Render Mode: Auto
  • Blazor Interactivity Location: Not sure
@SerratedSharp
Copy link
Author

SerratedSharp commented Aug 26, 2024

Looks like the dismiss issue is an unrelated known issue that cropped as a result of not including the default class(which this issue confirms ommitted the default offcanvas class is a normal part of implementing the responsive offcanvas. Known issue in BS:

twbs/bootstrap#36962

This is a jsfiddle showing a properly implemented responsive offcanvas which uses the data-bs-target workaround for the close button issue when the default class is not present:
https://jsfiddle.net/7yvdL5p4/1/

I will see if I can do a PR for this.

SerratedSharp added a commit to SerratedSharp/blazorbootstrap that referenced this issue Aug 26, 2024
…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.
@SerratedSharp SerratedSharp linked a pull request Aug 27, 2024 that will close this issue
@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.

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 a pull request may close this issue.

2 participants