Codecov Report

Attention: Patch coverage is 90.47619% with 2 lines in your changes missing coverage. Please review.

Project coverage is 91.79%. Comparing base (40ba0d5) to head (47c0a3c).
Report is 90 commits behind head on dev.

@@            Coverage Diff             @@
##              dev   #10596      +/-   ##
==========================================
+ Coverage   91.74%   91.79%   +0.05%     
==========================================
  Files         426      426              
  Lines       13379    13406      +27     
  Branches     2580     2579       -1     
==========================================
+ Hits        12274    12306      +32     
+ Misses        530      528       -2     
+ Partials      575      572       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

This may seem dumb, but what does it mean by FlexDirection.cs(1,1): error CHARSET: Fix file encoding. I have no idea what this wants me to fix.

I have no idea what this wants me to fix.

Just run dotnet format in scr folder, it will fix the encoding for you.

I don't think we should add this (was gonna answer on your issue but you beat me to it with the PR), MudStack is meant to be a simple little helper nothing more. I see very little gain in adding this complexity to it.

Someone added Wrapping functionality to MudStack, also makes no sense at all.

I don't think we should add this (was gonna answer on your issue but you beat me to it with the PR), MudStack is meant to be a simple little helper nothing more. I see very little gain in adding this complexity to it.

Someone added Wrapping functionality to MudStack, also makes no sense at all.

Its cool if this gets rejected or not, I just wanted to make the PR because if it gets officially rejected then I'll just make my own component that does this same thing.

I would like to say, if you do reject this component, at the very least consider adding the helper classes in the Styles/utilities/flexbox/_flex-direction.scss because really those are the core functionality of something like this and they are fairly generic and common in similar other libraries for control flex direction at different screen sizes. Its pretty much how I have been using MudStack for the last 3-4 months I just wanted to add the ability to include them as part of the documentation.

Though I do disagree that having no way to control when MudStack breaks from rows to columns other than a boolean is very limited. I have run into a few different cases in which I had to drop using the Row property and ended up just using my CSS classes instead because its nice that when your on a larger screen maybe something is in a row, but when it breaks down to a smaller size it drops to columns.

Also, I apologize if this or my statement below comes across as rude, that is not my intention. I understand that tone can sometimes be misinterpreted in text, as I've experienced many times myself. My only goal is to contribute positively to this project that I rely on, not to complicate it. These are changes I found necessary while working on our internal sites, and I thought they might be beneficial to the community as a whole.

As an example, this was using MudStack until v8 which fixed issues I had with MudToggleGroup But I basically had built this entire interface with MudStack using my css classes because I needed a way to tell the buttons to be columns at XS and rows at MD+

This still uses custom css even with MudToggleGroup because it has the same pitfall that MudStack does, its a boolean setting one way or the other not flexibility.

Because without those classes, this is pretty much what my UI looks like on a Mobile device because MudStack and MudToggleGroup are not flexible enough in my applications.

***I did plan on fixing MudToggleGroup to be responsive in a similar way after MudStack but if they are not supposed to work that way, I'll just keep using my custom css to fix it locally. ***

@Garderoben I think this could be worthwhile even if it's simplified to be just a single breakpoint that switches between row and column. There is definitely merit adding responsive design support

@Garderoben I think this could be worthwhile even if it's simplified to be just a single breakpoint that switches between row and column. There is definitely merit adding responsive design support

I am more than happy to discuss the syntax, but I would like to point out that current syntax is current the simplest I could implement. Which seems odd given that something like

<MudStack Breakpoint="Breakpoint.MdAndUp">

This has the advantage that it using an existing enum, which means one less needs to be created, it also means users are familiar with the syntax and how to use it. However, this syntax is far more logic to implement, and I believe has a much bigger issue of making the code harder to understand when being read.

Let's discuss the first part, complexity to implement. When using something like the existing Breakpoint enum, It means you have to support every variation of its current implementation otherwise it'll be confusing for users attempting to use it. meaning you would have to support [size], [size]AndDown, [size]AndUp, None and Always For a total of 16 different variations of breakpoint. Two of which are kinda of pointless. As Breakpoint.Always could just be done with Row="true | false" and the same is basically said for Breakpoint.None, so really 14 differen variations. This also means you would allow developers to do something like this

<MudStack Breakpoint="Breakpoint.Md">

Which to mean would mean do columns as its the default, until the breakpoint is met, but since the break point is Breakpoint.Md and not Breakpoint.MdAndUp it would only be a row at the breakpoint of Md. So basically, it would be columns at xs, sm, lg, xl, and xxl and a row at md

My next issue with a syntax like this is it makes whats happening a little more confusing to the user writing and maybe even a future developer working on the project.

Let's look at the two following examples:

<MudStack Breakpoint="Breakpoint.MdAndUp">

A developer who understands and has read how MudStack would know that by default MudStack is columns and this is saying, hey at the breakpoint of md and up to make it rows. But a new developer isn't really going to understand how this works. Its not clear what's happening at the breakpoint just that there is a breakpoint.

Now lets look at the next example:

<MudStack Breakpoint="Breakpoint.MdAndUp" Row="true">

In this example, the breakpoint is the same, however because Row is true then what happens is the MudStack defaults to rows until the breakpoint at which point it would switch to Columns. I find this confusing because it's not very clear to what breakpoint is doing. This logic also makes implementing this syntax a lot more complicated because not only are you checking what the breakpoint is, you have to determine if your doing Rows or Columns based on the Row attribute.

Now with all of this said, this was really the only alternative syntax someone came up with and I just find it to be far more complicated to implement and it makes reading what's happening harder to do. I am in NO WAY saying my syntax is the best, its simply the easier syntax I could implement with minor code changes that I felt made reading and understanding what's happening easy. I am happy to discuss alternative syntax's to find one that feels right for MudBlazor.

I do believe that making MudStack be responsive is only a bonus to MudBlazor and something worth looking into, if its not via attributes like this pull request, maybe at the very least making the classes available as part of the core library would be a great step in my opinion.

I have frequently experienced the aforementioned limitations and believe that the MudBlazor library could benefit from finding a viable solution to enhance MB's responsiveness, especially when transitioning from rows to columns. As a cross-platform MudBlazor user, I recognize this as a common issue among mobile device users. Integrating this complexity into the library at an early stage would be advantageous in my opinion, as responsive design should be an integral part of the library from the outset. If complexity poses a concern in MudStack, perhaps a dedicated component could be added to address this essential feature.

This syntax <MudStack Breakpoint="Breakpoint.MdAndUp"> is far superior from the user perspective IMO. Usability / ease of use should be number one priority, not how difficult the code to implement a syntax is. So if the team can come to a decision on accepting this PR I'd vote for going for this one instead. It is also consistent with how other responsive components work.

As for if we want this feature or not it is a resounding YES from my side. The value it provides is worth the additional complexity in MudStack IMO. And MudStack is one of the simplest components we have, so there is no risk that adding this could pose maintainability problems in the future.

Our success is that we offer functionality in an more "csharpy" kind of way which you could also do with just CSS but there are many who have a hard time with CSS and are glad to be able to take what we offer.

I am more than happy to implement either syntax. And honestly either syntax is not super hard. If the syntax needs to be changed for this to be implemented, I can have that updated in a few hours. Y'all just let me know what I need to make MudStack better. Any one on the discord knows I constantly rave about this component specifically.

@henon though you said

It is also consistent with how other responsive components work.

What other responsive components use the Breakpoint syntax? I've never seen it used outside of the example for the Breakpoint provider example in the docs?

What other responsive components use the Breakpoint syntax?

I think he is talking about:

• MudBreakpointProvider
• MudDrawer
• MudHidden
• MudTable
• MudDataGrid

And non component IBrowserViewportService.

They have breakpoint settings that uses the Breakpoint enum.

I did not realize all of those components had that Attribute. In that case, I also agree the syntax should be Breakpoint and I will just update it later today. I don't think I need to wait for a response, I fully agree the syntax should be consistent with other components.

Added some comments on the scss file, i think we already have all the utilities you would need for this?

And as discussed already if this can be simplified way more i could probably agree on adding it i prefer the syntax <MudStack Breakpoint="Breakpoint.MdAndUp"> but again MudStack was just meant to be a quick little helper, adding this would maybe not complicate this component at this stage but where do we draw the line? MudStack should NOT be a wrapper for flexbox

I have no problem updating the syntax to use <MudStack Breakpoint="Breakpoint.MdAndUp">. As for why this should be done, responsive web design is why. And yes, you can say just use the built in css classes and do

<MudStack Row Class="flex-md-column">
    <MudPaper Class="mud-theme-primary" Width="100%" Height="64px"/>
    <MudPaper Class="mud-theme-secondary" Width="100%" Height="64px"/>
    <MudPaper Class="mud-theme-secondary" Width="100%" Height="64px"/>
</MudStack>

My only argument with this statement is, in two years of using MudBlazor I have never noticed the small footnote of responsive flex classes pinned at the bottom of the utilities section of the docs with 0 examples of using them. So maybe this whole PR should just be, hey here is how you can make MudStack responsive in the docs and then literally just add this example:

<MudStack Row Class="flex-md-column">
    <MudPaper Class="mud-theme-primary" Width="100%" Height="64px"/>
    <MudPaper Class="mud-theme-secondary" Width="100%" Height="64px"/>
    <MudPaper Class="mud-theme-secondary" Width="100%" Height="64px"/>
</MudStack>

Because as I said, the only reason I want this is, because having a component that has no way to respond to the users screen size just seems crazy to me, but like you said, it can already be done... but how many other developers using MudStack didn't realize you already had the built in classes to use this way? I suspect I can safely assume more than just me had not found that section in the docs before today.

So maybe this PR isn't needed, but better documentation on how to make components more responsive like MudStack should be included in the docs. Like I said, maybe just throw in an example like the one above would would probably be fine.

I will say as a final note for discussion is, I still much prefer the idea of having Attributes like Breakpoint vs using CSS utility classes at least in my Blazor project it just feels better to use attributes like that. maybe its just me liking Attributes over CSS utility classes since I use Tailwind for all of my non blazor projects and all that is, is utility classes. Attributes make blazor feel like a component, not just styled css.

Honestly after thinking about it some more, I feel like a better way to just document components like MudStack would be to drop the examples of Row attribute and instead show examples using the CSS Utility functions

<MudStack Class="flex-row flex-md-column">
    <MudPaper Class="mud-theme-primary" Width="100%" Height="64px"/>
    <MudPaper Class="mud-theme-secondary" Width="100%" Height="64px"/>
    <MudPaper Class="mud-theme-secondary" Width="100%" Height="64px"/>
</MudStack>

This has the advantage that newer users coming in will see and be able to reference these utility classes and without pointing out the Row attribute it should slowly phase out over time. It also gives a good example of how to make the component responsive. And since your not remove the Row property, just not showing an example of it, it wont break anyone already using it.

Kind of a lie, I would still personally prefer an attribute like Breakpoint but am happy to concede as there is a valuable question of when do you make something an attribute and when do you tell people just use the already existing classes. I know that must be a fine line to walk. So even though I would personally prefer the breakpoint I understand the choice that has to be made here.

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@Garderoben Does this look more like what you were expecting. Sorry about the ping, just wanted haven't seen any movement in two weeks and wanted confirmation.

@Garderoben Does this look more like what you were expecting. Sorry about the ping, just wanted haven't seen any movement in two weeks and wanted confirmation.

Sorry i missed this, I'm back next week i can test it out then