Skip to content

Conversation

@ScarletKuro
Copy link
Member

@ScarletKuro ScarletKuro commented Oct 17, 2024

Description

Components should be as standalone as possible, otherwise, you risk creating a "monolith." Only basic base components should exist, such as MudComponentBase. The only exception is MudFormComponent, which should only contain logic related to validation (our attempts to handle conversion logic and dictates the Value property, but that’s off-topic).

Also, the name of the base class is weird, as NavLink is not really a select item.

Fixes: #5051

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 (apply this instead of enhancement) PR: needs review labels Oct 17, 2024
@codecov
Copy link

codecov bot commented Oct 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.05%. Comparing base (bb3759a) to head (c4f3c7a).
Report is 1 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev   #10045      +/-   ##
==========================================
- Coverage   91.05%   91.05%   -0.01%     
==========================================
  Files         410      410              
  Lines       12457    12454       -3     
  Branches     2428     2428              
==========================================
- Hits        11343    11340       -3     
  Misses        566      566              
  Partials      548      548              

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

@ScarletKuro
Copy link
Member Author

Btw Ripple is not even used in MudSelectItem, there are zero references to it. So, we can remove that as well, since it was essentially just copy-paste from MudBaseSelectItem. The Href cannot be set on MudSelectItem, nor can ForceLoad. Basically, all navigation properties are useless. Most importantly, the OnClickHandler is not used, as MudSelectItem uses OnClicked. I will remove that later, but this is why base classes are a pain for components.

@ScarletKuro
Copy link
Member Author

Btw Ripple is not even used in MudSelectItem, there are zero references to it. So, we can remove that as well, since it was essentially just copy-paste from MudBaseSelectItem. The Href cannot be set on MudSelectItem, nor can ForceLoad. Basically, all navigation properties are useless. Most importantly, the OnClickHandler is not used, as MudSelectItem uses OnClicked. I will remove that later, but this is why base classes are a pain for components.

Cleanup. Apparently even the OnClick eventcallback wasn't used and this PR also closes this issue: #5051 as it never ever implemented it, as as benno mentioned #5051 (comment) this is an API inconsistency.

@ScarletKuro
Copy link
Member Author

Added to v8.0.0 Migration Guide #9953

@ScarletKuro ScarletKuro merged commit a6055e6 into MudBlazor:dev Oct 18, 2024
@ScarletKuro ScarletKuro deleted the base_remove branch October 18, 2024 11:05
LLauter pushed a commit to cannellamedia/MudBlazor that referenced this pull request Dec 16, 2024
@kibblewhite
Copy link

Dang I was using this to create what was called a 'MudSelectOptGroup' which created 'optgroup' elements in the select input/dropdown element. Sad days... Will have to figure a new way around. Just updated to version 8, will revert until I can figure this one out.

@danielchalmers
Copy link
Member

Dang I was using this to create what was called a 'MudSelectOptGroup' which created 'optgroup' elements in the select input/dropdown element. Sad days... Will have to figure a new way around. Just updated to version 8, will revert until I can figure this one out.

You may want to copy MudBaseSelectItem into your own project if you don't want to reimplement the properties, it's a fairly small class.

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 (apply this instead of enhancement)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OnClick not triggered on MudSelectItem

3 participants