Skip to content

MudTabs: Add nullable annotation#9889

Merged
ScarletKuro merged 3 commits intoMudBlazor:devfrom
meenzen:refactor/mud-tabs-nullable
Oct 2, 2024
Merged

MudTabs: Add nullable annotation#9889
ScarletKuro merged 3 commits intoMudBlazor:devfrom
meenzen:refactor/mud-tabs-nullable

Conversation

@meenzen
Copy link
Contributor

@meenzen meenzen commented Oct 1, 2024

Description

Part of #6535

Let's get started with the Hacktoberfest efforts 🎉

How Has This Been Tested?

none

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 enhancement Adds a new feature or enhances existing functionality (not fixing a defect) in the main library PR: needs review labels Oct 1, 2024
@meenzen meenzen requested a review from ScarletKuro October 1, 2024 22:17
@ScarletKuro ScarletKuro requested a review from henon October 2, 2024 10:13
ScarletKuro
ScarletKuro approved these changes Oct 2, 2024
View reviewed changes
Copy link
Member

@ScarletKuro ScarletKuro left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM
Tho I'm not sure about the throw new InvalidOperationException addition:

  1. New behavior, but here it will most likely never throw
  2. Decreases code coverage as it's new code, and as I said in really scenario they are most likely never be nulls so I'd probably just suppress it with !

Adding @henon for second opinion

@ScarletKuro ScarletKuro added the hacktoberfest-accepted Issues and PRs which were accepted as Hacktoberfest submissions label Oct 2, 2024
@henon
Copy link
Contributor

henon commented Oct 2, 2024

The way I see it PanelRef can only be null if someone were to remove the @ref from the razor. Then it would cause unit tests to fail which is a good thing. So I think this is worth the loss in coverage.

@henon
Copy link
Contributor

henon commented Oct 2, 2024

But on the other hand, if suppressed with ! it would just cause a NullRef exception, so I guess that suffices.

@meenzen
Copy link
Contributor Author

meenzen commented Oct 2, 2024

I've updated the code to suppress the the nullability warnings instead of throwing exceptions

@ScarletKuro
Copy link
Member

ScarletKuro commented Oct 2, 2024 •
edited
Loading

Before I click merge, can you make sure that the PR shows up in the hackforthebest website?
upd: nvm, just saw your discord reply.

@codecov
Copy link

codecov bot commented Oct 2, 2024 •
edited
Loading

Codecov Report

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

Project coverage is 90.82%. Comparing base (28bc599) to head (c46a39f).
Report is 1103 commits behind head on dev.

Files with missing lines Patch % Lines
src/MudBlazor/Components/Tabs/MudTabs.razor.cs 78.57% 1 Missing and 2 partials ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##              dev    #9889      +/-   ##
==========================================
+ Coverage   89.82%   90.82%   +0.99%     
==========================================
  Files         412      406       -6     
  Lines       11878    12830     +952     
  Branches     2364     2488     +124     
==========================================
+ Hits        10670    11653     +983     
+ Misses        681      623      -58     
- Partials      527      554      +27

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ScarletKuro ScarletKuro merged commit 589f4b3 into MudBlazor:dev Oct 2, 2024
@ScarletKuro ScarletKuro mentioned this pull request Oct 2, 2024
Open
94 tasks
@ScarletKuro
Copy link
Member

ScarletKuro commented Oct 2, 2024

Thanks!

@meenzen meenzen deleted the refactor/mud-tabs-nullable branch October 2, 2024 11:43
@ZizWing ZizWing mentioned this pull request Oct 11, 2024
Closed
4 tasks
ScarletKuro added a commit to ScarletKuro/MudBlazor that referenced this pull request Oct 11, 2024
@ScarletKuro
MudTabs: Fix ElementReference regression in MudBlazor#9889
9d6dc56
@ScarletKuro ScarletKuro mentioned this pull request Oct 11, 2024
Merged
7 tasks
ScarletKuro added a commit that referenced this pull request Oct 11, 2024
@ScarletKuro
MudTabs: Fix ElementReference regression in #9889 (#9970)
84d5509
ScarletKuro added a commit that referenced this pull request Oct 11, 2024
@ScarletKuro
MudTabs: Fix ElementReference regression in #9889 (#9970)
6952880 
(cherry picked from commit 84d5509)
LLauter pushed a commit to cannellamedia/MudBlazor that referenced this pull request Dec 16, 2024
@ScarletKuro @LLauter
MudTabs: Fix ElementReference regression in MudBlazor#9889 (MudBlazor…
09c1326 
…#9970)
@danielchalmers danielchalmers added regression Previously worked and now doesn't and removed accidental break labels Jul 1, 2025
@dependabot dependabot bot mentioned this pull request Aug 7, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@henon henon henon approved these changes

@ScarletKuro ScarletKuro ScarletKuro approved these changes

Assignees

No one assigned

Labels

enhancement Adds a new feature or enhances existing functionality (not fixing a defect) in the main library hacktoberfest2024 hacktoberfest-accepted Issues and PRs which were accepted as Hacktoberfest submissions regression Previously worked and now doesn't

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@meenzen @henon @ScarletKuro @danielchalmers