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.84k stars 373 forks source link

fix: MessageBar has the close button [X] in static ssr #962

Closed iotalambda closed 12 months ago

iotalambda commented 1 year ago

🐛 Bug Report

MessageBar has the close button [X] in static ssr, but it doesn't do anything

🀔 Expected Behavior

Dont show [X]

vnbaaij commented 1 year ago

I don't think this is what we would want to do in general.

A MessageBar for example is supposed to have a close button. If you want to use it in your app, then it needs interactivity (either Server or WebAssembly). Removing the button is not the right course of ACTION.

@dvoituron , @danroth27, thoughts?

dvoituron commented 1 year ago

I think, the only case where the close button could be removed is when using the component without the MessageService. But with the service, this button should be there.

dvoituron commented 1 year ago

We could add a property ˋShowCloseButton` but that's all. No?

iotalambda commented 1 year ago

Use case: I use blazor server, but having static ssr & streaming on my external pages, especially on the login page, is really nice. Circuits are created only for logged in users. I think MessageBar fits very well in displaying login errors and such. And I don't mind it being non-interactive.

@dvoituron that would work!

danroth27 commented 12 months ago

Why would you ever want to hide the close button? How does that help? How do you ever dismiss the message bar? It seems like the right fix is to enable interactivity. Or are you saying there's a valid scenario for displaying a message bar without a close button and the message bar should support that independent of whether the message bar is rendered interactively?

vnbaaij commented 12 months ago

The Fluent design specs also say that a close button is required. I don't think we should want to change the components just to get (more) SSR support. If a component does not work with it, use interactivity. We now have these 'islands of interactivity' at or disposal. Why not use them?

Use case: I use blazor server, but having static ssr & streaming on my external pages, especially on the login page, is really nice. Circuits are created only for logged in users. I think MessageBar fits very well in displaying login errors and such. And I don't mind it being non-interactive.

When using redermode InteractiveAuto, you do get a circuit, but it will be closed if it is no longer needed/used.

vnbaaij commented 12 months ago

We could add a property ˋShowCloseButton` but that's all. No?

I would say "no" as it would deviate from the Fluent specs just to enable an SRR scenario

iotalambda commented 12 months ago

The Fluent design specs also say that a close button is required. I don't think we should want to change the components just to get (more) SSR support. If a component does not work with it, use interactivity. We now have these 'islands of interactivity' at or disposal. Why not use them?

Use case: I use blazor server, but having static ssr & streaming on my external pages, especially on the login page, is really nice. Circuits are created only for logged in users. I think MessageBar fits very well in displaying login errors and such. And I don't mind it being non-interactive.

When using redermode InteractiveAuto, you do get a circuit, but it will be closed if it is no longer needed/used.

I'd prefer not using any interactivity for my public facing pages, as that would expose an attack vector of creating potentially thousands of circuits, no? I considered using a WebAssembly Client for this, but then the only use case would be these interactive MessageBars, so I probably won't go there :D My static ssr login page works otherwise perfectly (thanks for the template!), so I will probably just hack the [X]'s away with some CSS. I do understand the reason for voting "no", though!

vnbaaij commented 12 months ago

A MessageBar is just not a very suitable component for what you want to use it for. From the specs:

A message bar communicates important information about the state of the entire product or surface—for example, the status of a page, panel, dialog, or card. The information shouldn’t require someone to take immediate action and can never be used to upsell or advertise.

If there is a login error, I would argue immediate action is required.

Closing this as no action needed from our side

iotalambda commented 12 months ago

A MessageBar is just not a very suitable component for what you want to use it for. From the specs:

A message bar communicates important information about the state of the entire product or surface—for example, the status of a page, panel, dialog, or card. The information shouldn’t require someone to take immediate action and can never be used to upsell or advertise.

If there is a login error, I would argue immediate action is required.

Closing this as no action needed from our side

Is this exact spec available online somewhere? Can't find it by binging or gpting.

It also does seem that in other implementations the close button (MessageBar actions) is configurable or even opt-in in react... https://developer.microsoft.com/en-us/fluentui?fabricVer=8#/controls/web/messagebar https://react.fluentui.dev/?path=/docs/components-messagebar--default

vnbaaij commented 12 months ago

The Figma files can be found here: https://aka.ms/Fluent2Toolkits/Web/Figma

React v8 (your first link) targeted a previous version of the Fluent Design (v1). The second link is the currren implementaton (targeting Fluent v2)