Table: Avoid checkbox overflow

[danielchalmers]

Description

Resolves #9839 by using the Dense padding for checkboxes in a table, removing the overflowing hover effect that caused a scrollbar to appear. This includes the MudDataGrid.

See it here: https://try.mudblazor.com/snippet/mOcIvkbybOVthUow

How Has This Been Tested?

visually

Type of Changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation (fix or improvement to the website or code docs)

Vertical height of both tables are identical and width was manually set to 400px to keep consistency, making the dimensions of both in the screenshots below equal to 400px by 186px.

Number 4 has mouse over in both screenshots

Before

After

Checklist

• The PR is submitted to the correct branch (dev).
• My code follows the code style of this project.
• I've added relevant tests.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.94%. Comparing base (28bc599) to head (4f78274).
Report is 581 commits behind head on dev.

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #9914      +/-   ##
==========================================
+ Coverage   89.82%   90.94%   +1.11%     
==========================================
  Files         412      409       -3     
  Lines       11878    12524     +646     
  Branches     2364     2446      +82     
==========================================
+ Hits        10670    11390     +720     
+ Misses        681      579     -102     
- Partials      527      555      +28     

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

[igotinfected]

From my POV the hover/focus styling with the default colour becomes practically invisible, I have to focus on it to be able to see it :(

Had a peek at other libraries:

• MUI hides overflow
• others don't have a hover effect on the checkbox to begin with
• fluent ui has focus styling on the checkbox cell rather than the checkbox

overflow: hidden seems to work and keeps the hover/focus effects readable, what do you think? https://try.mudblazor.com/snippet/GOQSPOYTBVqnDEwI
Could even consider making the checkboxes Size.Small for an even denser look as a bonus

[danielchalmers]

@igotinfected Yeah I'm not super satisfied with it but likely the simplest solution.

1. Hiding overflow could drastically change user layouts (and should be applied on all in that case, not only ones with checkboxes)
2. Clipping the content avoids fixing the root issue
3. Making the checkboxes small would have to be done on the user's end so would break a lot of tables
4. A small checkbox still has the same negative margin issue

But if MUI clips it then it's worth considering. You can find a similar long-standing issue with dialogs here: https://try.mudblazor.com/snippet/QEmIbkYpLtxRseHO

[dennisrahmen]

@danielchalmers could you fix this for the DataGrid too? 🙏

[henon]

I think this is one of those decisions @Garderoben can help us with.

[danielchalmers]

@danielchalmers could you fix this for the DataGrid too? 🙏

@dennisrahmen Could you send me a repro link? I'm not super well versed with the DataGrid

[dennisrahmen]

@danielchalmers here you go: https://try.mudblazor.com/snippet/mYQybkkAJCedBTIs
Thanks for taking a look.

[danielchalmers]

@danielchalmers here you go: https://try.mudblazor.com/snippet/mYQybkkAJCedBTIs Thanks for taking a look.

@dennisrahmen It's actually using the same styles so it works out of the box with this PR. Thanks for helping me find this side effect!

[henon]

Is it possible to solve the scrollbar problem without making the focus circle smaller?

[dennisrahmen]

@henon as it is right now the focus circles of each row are overlapping. So you are clicking in row number two but the checkbox in row number three would be selected, especially problematic with touch devices.

You could keep the circle size for the normal view and only make it smaler when the DataGrid/Table is set to Dense="true"

[henon]

OK, so I see two possible solutions. Either configure the circle to be so small that it doesn't overflow the row or make it so that the row cuts off the circle to avoid overflow onto other rows.

[danielchalmers]

OK, so I see two possible solutions. Either configure the circle to be so small that it doesn't overflow the row or make it so that the row cuts off the circle to avoid overflow onto other rows.

In general I don't think controls should exceed their space, even just the hover effects. Looks weird

[henon]

In gmail they made the circle fit the table row exactly

[danielchalmers]

In gmail they made the circle fit the table row exactly

Would work great but our rows are dynamically sized while theirs are a set height

[henon]

Looks like the circumstances are forcing us to compromise somewhere ;). Either make the circle customizable so it can be set to the minimal row height or accept that the circle can be cut off. I don't think that the hover circle should be so small that it touches the corners of the checkbox. That practically defeats the purpose of it.

[dennisrahmen]

Can't we assume a reasonable default here?

Like setting padding to 5px instead of 0?
That would look like this:

I would say for nearly all DataGrids the smallest it can be is just text in all columns without any Typography applied and the DataGrid/Table in Dense mode.

[danielchalmers]

@dennisrahmen 5px causes an increase in the height of the original example https://try.mudblazor.com/snippet/QaQRuBcfWQyOUycL (unless you're talking about changing the style in another place)

[danielchalmers]

There is already a precedent for this with the Dense property

https://mudblazor.com/components/checkbox#dense

[danielchalmers]

@dennisrahmen @henon See description for new proposal.

Now uses the dense padding. Still uses a negative margin but this should be suitable for almost all cases and a definite improvement over the current behavior.

[henon]

Awesome. Can we get a final screenshot for the historical record please? Light, if you will, because in dark mode the hover circle ist almost invisible.

[danielchalmers]

Awesome. Can we get a final screenshot for the historical record please? Light, if you will, because in dark mode the hover circle ist almost invisible.

I think you are looking for the second screenshot in the updated description @henon

[henon]

Thanks @danielchalmers !