search

microsoft / fluentui-blazor

Microsoft Fluent UI Blazor components library. For use with ASP.NET Core Blazor applications
https://www.fluentui-blazor.net
MIT License
3.87k stars 376 forks source link

fix: Popover inherits parent's style #1956

Closed verdie-g closed 6 months ago

verdie-g commented 6 months ago

🐛 Bug Report

The content of a Popover inherits the style of the parent. For example, the content would be bold if the parent is FluentHeader.

💻 Repro or Code Sample

<FluentLayout>
    <FluentHeader>
        DotnetEventViewer
        <FluentSpacer />
        <FluentButton Id="trace-metadata">
            <!-- ... -->
        </FluentButton>
        <FluentPopover Style="width: 300px; background-color: var(--accent-fill-rest)" AnchorId="trace-metadata">
            <!-- ... -->
        </FluentPopover>
    </FluentHeader>
    <!-- ... -->
</FluentLayout>

Full code: https://github.com/verdie-g/dotnet-event-viewer/blob/02526a5a9b40eeb9ff3f67a11b56351ff431e58c/DotnetEventViewer/Components/TraceMetadata.razor

🤔 Expected Behavior

I suppose I would expect the Popover to auto-magically be appended to the <body> instead so it inherits nothing. Not sure how other frameworks deal with that.

😯 Current Behavior

Like any other component the Popover is added where it was defined in the DOM so it inherits the style of its parents.

image

🌍 Your Environment

verdie-g commented 6 months ago

Ah might be fixed by FixedPlacement https://github.com/microsoft/fluentui-blazor/issues/1879 ?

Fixed placement allows the region to break out of parent containers.

vnbaaij commented 6 months ago

No, FixedPlacement won't help for this. I tried some stuff. Ended up with:

image

Added this

<style>
    .reset {
        all: revert;
    }
</style>

And did this:

<FluentPopover Class="reset" Style="width: 300px; " AnchorId="trace-metadata" @bind-Open="_visible" FixedPlacement="true">
    <Header><FluentLabel Typo="Typography.H3">My Header</FluentLabel></Header>
    <Body>
        <div class="body">
        <ul>
            <li>Bla</li>
            <li>Bla</li>
            <li>Bla</li>
            <li>Bla</li>
        </ul>
        </div>
    </Body>
</FluentPopover>

The class body on th Body element does not exist yet. I created it by changing the reboot.css so that .body is added to the body declaration. I can make that chnge permanenet for the next release. For now you would need to add to your style

.body {
    margin: 0;
    font-family: var(--body-font);
    font-size: var(--type-ramp-base-font-size);
    line-height: var(--type-ramp-base-line-height);
    font-weight: var(--font-weight);
    color: var(--neutral-foreground-rest);
    background-color: var(--neutral-fill-layer-rest);
    -webkit-text-size-adjust: 100%;
    -webkit-tap-highlight-color: rgba(0, 0, 0, 0);
}

Is adding the .body interesting, you think?

verdie-g commented 6 months ago

That looks hacky to me. I think the standard solution is to append the Popover to the <body>. For example, that's what MudBlazor did with their Popover. They have a popover-provider as a child of the body for all the popovers of the page.

image

vnbaaij commented 6 months ago

Yeah, that is just not how our Popover works now. I wouldn't call it a standard, but we do this for other components as well (dialog, messagebar, ...). Might make for an alternative approach in the future.

vnbaaij commented 6 months ago

For the next release the .body class has been added. Totally up to you if you want to use it, of course. I think it is handy to have that nevertheless.

Closing this one as a workaround is available