Skip to content

DataGrid: Change default values of TemplateColumn#9142

Merged
henon merged 4 commits intoMudBlazor:devfrom
dennisrahmen:datagrid-template-column-defaults
Jun 10, 2024
Merged

DataGrid: Change default values of TemplateColumn#9142
henon merged 4 commits intoMudBlazor:devfrom
dennisrahmen:datagrid-template-column-defaults

Conversation

@dennisrahmen
Copy link
Contributor

@dennisrahmen dennisrahmen commented Jun 8, 2024

Description

The TemplateColumn is generally used to add "Action" columns like editing, select and hierarchy.
Currently, the defaults are just the same as for the normal column, which is suboptimal.
In the most common use cases most features are not used or need to be setup first anyway, like with SortBy.

Some of these features like drag and drop add spacing to the column, which is particularly noticeable for select and hierarchy columns.

Furthermore, some features could not been turned off per column, like drag and drop for the hierarchy and select columns.

I also changed the default for 'ShowInFooter' for the SelectColumn since I have seen multiple people complain about the hole row just for the one checkbox.
I would remove that completely to be honest, no other columns support this, and you could always make the header sticky with 'FixedHeader' and have a better experience.

Examples of space savings:
image

image

image

image

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)

Checklist

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

@github-actions github-actions bot added breaking change This change will require consumer code updates PR: needs review labels Jun 8, 2024
@dennisrahmen
Copy link
Contributor Author

If this PR gets merged the following default values have changed to false and need to be added to the migration guide:

Template Column

  • ShowColumnOptions
  • Resizable
  • DragAndDropEnabled
  • Sortable
  • Filterable

Select Column

  • DragAndDropEnabled (change inherited from Template column)
  • ShowInFooter

Hierarchy Column (changes inherited from Template column)

  • DragAndDropEnabled

@dennisrahmen
Copy link
Contributor Author

@tjscience last year we talked about this change here: #6049 (comment)
I hope we had the same thing in mind by setting sensible defaults.

@codecov
Copy link

codecov bot commented Jun 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.69%. Comparing base (28bc599) to head (a560694).
Report is 260 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #9142      +/-   ##
==========================================
+ Coverage   89.82%   90.69%   +0.86%     
==========================================
  Files         412      399      -13     
  Lines       11878    12465     +587     
  Branches     2364     2427      +63     
==========================================
+ Hits        10670    11305     +635     
+ Misses        681      623      -58     
- Partials      527      537      +10     

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

@dennisrahmen
Copy link
Contributor Author

@henon @danielchalmers @ScarletKuro ready for review ✌️

@ScarletKuro ScarletKuro requested a review from tjscience June 8, 2024 07:32
@ScarletKuro
Copy link
Member

ScarletKuro commented Jun 8, 2024

I will let @tjscience to decide, I'm unsure about the ShowInFooter="false" default as it leaves awkward space in footer, also it's been default for a very long time. I only pretty much agree with Fiterable="false" as TemplateColumn is not attached to any data and filtering is not possible as result confusing people.
It's not only for action, but for dynamic usage too when you don't have a property at design time.

Copy link
Contributor

@tjscience tjscience left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think everything looks good here. Thanks, @dennisrahmen!

@ScarletKuro ScarletKuro requested a review from henon June 8, 2024 14:14
@dennisrahmen
Copy link
Contributor Author

@ScarletKuro the "awkward space in footer" was there already and is not even dependent on the ShowInFooter setting, as it is introduced though the spacing of the checkboxes.

I wanted to open a separate pr for that.

@ScarletKuro
Copy link
Member

@dennisrahmen can you write down all breaking change of that PR for the #8447
in similar format so we could just copy paste it? Thanks.

@dennisrahmen
Copy link
Contributor Author

@ScarletKuro @henon I took the hole block so you can just copy paste.

## MudDataGrid

- Removed `FilterDefinition.GenerateFilterExpression()` replace with `FilterExpressionGenerator.GenerateExpression`
- Removed other old APIs from `FilterDefinition` that was replaced by `IFilterDefinition` API.
- `FilterContext.FilterActions` all properties are marked as required.
- `FooterContext.FooterActions` all properties are marked as required.
- `HeaderContext.SetSelectAllAsync` all properties are marked as required.
- `HeaderContext.IsAllSelected` changed type from `bool` to `bool?`.
- **Column**: replace `IsEditable` with `Editable`
- **TemplateColumn**
  - `ShowColumnOptions` changed default to false
  - `Resizable` changed default to false
  - `DragAndDropEnabled` changed default to false
  - `Sortable` changed default to false
  - `Filterable` changed default to false
- **SelectColumn**
  - `DragAndDropEnabled` changed default to false
  - `ShowInFooter` changed default to false
- **HierarchyColumn**
  - `DragAndDropEnabled` changed default to false

For more details see https://github.com/MudBlazor/MudBlazor/pull/8548, https://github.com/MudBlazor/MudBlazor/pull/8317, https://github.com/MudBlazor/MudBlazor/pull/8892, https://github.com/MudBlazor/MudBlazor/pull/9142

@henon henon mentioned this pull request Jun 10, 2024
@henon henon merged commit 1dba05b into MudBlazor:dev Jun 10, 2024
@henon
Copy link
Contributor

henon commented Jun 10, 2024

Thanks Dennis

@dennisrahmen dennisrahmen deleted the datagrid-template-column-defaults branch June 10, 2024 11:32
Elodon added a commit to Elodon/MudBlazor that referenced this pull request Jun 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking change This change will require consumer code updates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants