Skip to content

MudDataGrid: Remove redundant column menu (#7566)#7627

Merged
henon merged 3 commits intoMudBlazor:devfrom
MohamedYassin-J:fix/datagrid-redundant-menu
Oct 28, 2023
Merged

MudDataGrid: Remove redundant column menu (#7566)#7627
henon merged 3 commits intoMudBlazor:devfrom
MohamedYassin-J:fix/datagrid-redundant-menu

Conversation

@MohamedYassin-J
Copy link
Contributor

@MohamedYassin-J MohamedYassin-J commented Oct 10, 2023 •
edited by henon
Loading

Description

In the MudDataGrid component, I observed a redundant "⋮" menu being rendered when FilterMode is set to DataGridFilterMode.ColumnFilterRow and SortMode is set to SortMode.None.

This update rectifies this oversight by ensuring the column menu is not rendered with the aforementioned combination of properties.

Linked Issue:

How Has This Been Tested?

  1. Automated Testing:
       - Executed all relevant unit tests, all of which passed successfully.
       - Introduced tests within DataGridRedundantMenuTest.razor to validate the removal of the redundant menu based on specific FilterMode and SortMode combinations.

  2. Manual Testing:
       - Interacted with the MudDataGrid on the DataGridRedundantMenuTest.razor page, confirming the removal of the redundant menu with the specified conditions.

Types 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)

Visual Demonstrations

To visually highlight the addressed issue and its resolution:

Before the Fix:

GIF showing the redundant menu before the fix

After the Fix:

GIF showing the grid without the redundant menu after the fix

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 Oct 10, 2023
@codecov
Copy link

codecov bot commented Oct 10, 2023 •
edited
Loading

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (2279d75) 90.60% compared to head (d0c5bb8) 90.57%.
Report is 2 commits behind head on dev.

Additional details and impacted files 
@@            Coverage Diff             @@
##              dev    #7627      +/-   ##
==========================================
- Coverage   90.60%   90.57%   -0.03%     
==========================================
  Files         427      427              
  Lines       15218    15225       +7     
==========================================
+ Hits        13788    13790       +2     
- Misses       1430     1435       +5
Files Coverage Δ
.../MudBlazor/Components/DataGrid/HeaderCell.razor.cs 72.58% <100.00%> (+0.29%) ⬆️

... and 2 files with indirect coverage changes

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

@ScarletKuro
Copy link
Member

ScarletKuro commented Oct 10, 2023 •
edited
Loading

I don't see the DataGridRedundantMenuTest.razor to be used anywhere for test on bUnit side.
Adding `.razor. file is not enough, you need to use the created file explicitly here https://github.com/MudBlazor/MudBlazor/blob/dev/src/MudBlazor.UnitTests/Components/DataGridTests.cs
and check via html that dots are not present anymore.

ScarletKuro
ScarletKuro reviewed Oct 10, 2023
View reviewed changes
@ScarletKuro ScarletKuro requested a review from tjscience October 10, 2023 21:18
@ScarletKuro
Copy link
Member

ScarletKuro commented Oct 10, 2023

@henon @tjscience I don't think this is breaking change? As I understand the 3 dots are just visually redundant?

@MohamedYassin-J
Copy link
Contributor Author

MohamedYassin-J commented Oct 10, 2023

@henon @tjscience I don't think this is breaking change? As I understand the 3 dots are just visually redundant?

Yes, they are. So is it a bug fix or feature?

@MohamedYassin-J
Copy link
Contributor Author

MohamedYassin-J commented Oct 10, 2023

I don't see the DataGridRedundantMenuTest.razor to be used anywhere for test on bUnit side.
Adding `.razor. file is not enough, you need to use the created file explicitly here https://github.com/MudBlazor/MudBlazor/blob/dev/src/MudBlazor.UnitTests/Components/DataGridTests.cs
and check via html that dots are not present anymore.

Ok, I will.

@MohamedYassin-J
Copy link
Contributor Author

MohamedYassin-J commented Oct 11, 2023

I don't see the DataGridRedundantMenuTest.razor to be used anywhere for test on bUnit side. Adding `.razor. file is not enough, you need to use the created file explicitly here https://github.com/MudBlazor/MudBlazor/blob/dev/src/MudBlazor.UnitTests/Components/DataGridTests.cs and check via html that dots are not present anymore.

done @ScarletKuro.

@ScarletKuro
Copy link
Member

ScarletKuro commented Oct 11, 2023

Thanks. Now I will leave the review to @tjscience

@tjscience
Copy link
Contributor

tjscience commented Oct 28, 2023

This looks good. Thanks, @MohamedYassin-J!

@ScarletKuro
Copy link
Member

ScarletKuro commented Oct 28, 2023

@henon do we merge this? This doesn't look like as a breaking chance to me, but I need second opinion.

@henon henon added bug Unexpected behavior or functionality not working as intended and removed breaking change This change will require consumer code updates labels Oct 28, 2023
@henon henon merged commit 4e02b39 into MudBlazor:dev Oct 28, 2023
@henon
Copy link
Contributor

henon commented Oct 28, 2023

Thanks @MohamedYassin-J

@MohamedYassin-J MohamedYassin-J deleted the fix/datagrid-redundant-menu branch October 28, 2023 18:42
ilovepilav pushed a commit to ilovepilav/MudBlazor that referenced this pull request Nov 25, 2023
@MohamedYassin-J @ilovepilav
MudDataGrid: Remove redundant column menu (MudBlazor#7566) (MudBlazor…
8716e9b 
…#7627)

* MudDataGrid: Remove redundant column menu (MudBlazor#7566)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ScarletKuro ScarletKuro ScarletKuro left review comments

+1 more reviewer

@tjscience tjscience tjscience approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

bug Unexpected behavior or functionality not working as intended

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Remove redundant column menu in MudDataGrid

4 participants

@MohamedYassin-J @ScarletKuro @tjscience @henon