Skip to content

CssBuilder, StyleBuilder: Fix method with optional parameter is hidden by overload#8769

Merged
henon merged 7 commits intoMudBlazor:devfrom
ScarletKuro:cssbuilder_fix
Apr 21, 2024
Merged

CssBuilder, StyleBuilder: Fix method with optional parameter is hidden by overload#8769
henon merged 7 commits intoMudBlazor:devfrom
ScarletKuro:cssbuilder_fix

Conversation

@ScarletKuro
Copy link
Member

@ScarletKuro ScarletKuro commented Apr 21, 2024

Description

The problem with this is that if you have overloads:

StyleBuilder AddStyle(string style)
StyleBuilder AddStyle(string style, bool when = true)

And someone of them has an optional value bool when = true this two start to conflict with each other and code analyzers will tell you to fix it.
To fix it, there is two ways:

  1. Remove the one with least parameters and combine it with the optional one
  2. Remove optional parameter value

I went for second option.

More info: https://stackoverflow.com/questions/16546831/overloaded-methods-give-method-with-optional-parameter-is-hidden-by-overload-w

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 bug Unexpected behavior or functionality not working as intended PR: needs review labels Apr 21, 2024
@ScarletKuro ScarletKuro changed the title CssBuilder, StyleBuilder: Fix optional overloads hiding each other + … CssBuilder, StyleBuilder: Fix method with optional parameter is hidden by overload Apr 21, 2024
@codecov
Copy link

codecov bot commented Apr 21, 2024

Codecov Report

Attention: Patch coverage is 91.17647% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 90.12%. Comparing base (28bc599) to head (f96a190).
Report is 94 commits behind head on dev.

Files Patch % Lines
src/MudBlazor/Utilities/CssBuilder.cs 86.66% 0 Missing and 2 partials ⚠️
src/MudBlazor/Utilities/StyleBuilder.cs 94.73% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #8769      +/-   ##
==========================================
+ Coverage   89.82%   90.12%   +0.29%     
==========================================
  Files         412      419       +7     
  Lines       11878    12186     +308     
  Branches     2364     2400      +36     
==========================================
+ Hits        10670    10983     +313     
+ Misses        681      661      -20     
- Partials      527      542      +15     

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

@ScarletKuro
Copy link
Member Author

Improved missing xml comments and improved the wording in general, since it's basically a core part for building components.

@ScarletKuro
Copy link
Member Author

I think it would be interesting to add an implicit operator to not call build, what others think about it?

@ScarletKuro
Copy link
Member Author

Improved coverage, I'm done with this

@henon henon merged commit 9c49a1c into MudBlazor:dev Apr 21, 2024
@henon
Copy link
Contributor

henon commented Apr 21, 2024

Nice, looks like a lot of work went into the xml docs.

@ScarletKuro ScarletKuro deleted the cssbuilder_fix branch April 21, 2024 16:51
biegehydra pushed a commit to biegehydra/MudBlazor that referenced this pull request Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Unexpected behavior or functionality not working as intended

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants