Skip to content

Radio, Check Box, Switch: Unification and improved alignaments#9472

Merged
henon merged 25 commits intoMudBlazor:devfrom
ralvarezing:fix/switch-margin-right-missing
Oct 18, 2024
Merged

Radio, Check Box, Switch: Unification and improved alignaments#9472
henon merged 25 commits intoMudBlazor:devfrom
ralvarezing:fix/switch-margin-right-missing

Conversation

@ralvarezing
Copy link
Copy Markdown
Member

@ralvarezing ralvarezing commented Jul 21, 2024

Description

  • MudRadio, MudCheckBox, MudSwitch: These compoents now implements MudBooleanInput, the alignment was standarize so it follows all components norm and apeareance parameters were introduced like LabelPlacement #9472

Unification of the MudBooleanInput components: Checkbox, Radio, Switch.
Improvement of Margins and alignment.
Added Placement for Checkbox and Switch.

fixes #9009

LTR
image

RTL
image

Switch and Checkbox with Placement
image
image

How Has This Been Tested?

  • Added examples in AllInputsTest test page
  • Compile and execute Docs server project, explore Checkbox, Radio and Switch pages
  • Switch between Placement options

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 the breaking change This change will require consumer code updates (ex: removes/changes a public API) label Jul 21, 2024
@ralvarezing
Copy link
Copy Markdown
Member Author

Fixing all related unit tests...

btw. I took the liverty to add a RTL switch in the Tests.Viewer project :)

Comment thread src/MudBlazor/Components/Radio/MudRadioGroup.razor
@ralvarezing ralvarezing marked this pull request as ready for review July 22, 2024 04:29
@codecov
Copy link
Copy Markdown

codecov bot commented Jul 22, 2024

Codecov Report

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

Project coverage is 91.07%. Comparing base (bb3759a) to head (0948ad4).
Report is 2 commits behind head on dev.

Files with missing lines Patch % Lines
src/MudBlazor/Base/MudBooleanInput.cs 77.77% 1 Missing and 1 partial ⚠️
src/MudBlazor/Components/Radio/MudRadio.razor.cs 96.55% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #9472      +/-   ##
==========================================
+ Coverage   91.05%   91.07%   +0.01%     
==========================================
  Files         410      409       -1     
  Lines       12457    12446      -11     
  Branches     2428     2422       -6     
==========================================
- Hits        11343    11335       -8     
  Misses        566      566              
+ Partials      548      545       -3     

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

@ralvarezing
Copy link
Copy Markdown
Member Author

There, ready for review! 💪

Copy link
Copy Markdown
Member

@danielchalmers danielchalmers left a comment

Choose a reason for hiding this comment

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

Looking good at first glance, thanks for taking this on! We should be able to do deeper reviews when v8 starts rolling, next month most likely.

Comment thread src/MudBlazor.Docs/MudBlazor.Docs.csproj Outdated
Comment thread src/MudBlazor.sln Outdated
Comment thread src/MudBlazor/Components/CheckBox/MudCheckBox.razor.cs Outdated
@danielchalmers
Copy link
Copy Markdown
Member

danielchalmers commented Oct 5, 2024

This should be able to get a deeper review soon with v8 on its way

Copy link
Copy Markdown
Member

@danielchalmers danielchalmers left a comment

Choose a reason for hiding this comment

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

Briefly looked over this again, really like the refactor. Just a small note on the state method names

Comment thread src/MudBlazor/Base/MudBooleanInput.cs Outdated
Comment thread src/MudBlazor/Base/MudBooleanInput.cs Outdated
GetDisabled => GetDisabledState
GetReadOnly => GetReadOnlyState
@henon henon merged commit 30dc324 into MudBlazor:dev Oct 18, 2024
@henon
Copy link
Copy Markdown
Contributor

henon commented Oct 18, 2024

@ralvarezing We need a description for the v8 Migration Guide at #9953 for this PR please

@ScarletKuro
Copy link
Copy Markdown
Member

ScarletKuro commented Oct 18, 2024

If this gets merged in v8, pls sum up all the changes in similar format so I could copy-paste: #9953

Please sum up what needs to be added to guide

upd @henon haha

@henon henon mentioned this pull request Oct 18, 2024
@ralvarezing
Copy link
Copy Markdown
Member Author

@henon @ScarletKuro I don't know how are you organized to write the migration guide, so I'll leave a description here and tell me in any case.

  • MudRadio, MudCheckBox, MudSwitch: These compoents now implements MudBooleanInput, the alignment was standarize so it follows all components norm and apeareance parameters were introduced like LabelPlacement #9472

@Yomodo
Copy link
Copy Markdown
Contributor

Yomodo commented Oct 19, 2024

@ralvarezing Thanks, great work!
I believe I found a couple more.

Disabled Switch: Label is also disabled.
I think the label should be enabled.
image

Disabled Checkbox:
The label is enabled, which I think is correct.
However, hovering above the label will show a hand cursor.
image

@henon
Copy link
Copy Markdown
Contributor

henon commented Oct 19, 2024

@ralvarezing the description in the migration guide is not a summary of what your PR does but rather what exactly is the breaking change and what they have to change in their code to migrate. For example: Renamed property from "abc" to "xyz" or removed "abc" use "xyz" instead, or similar.

@ralvarezing
Copy link
Copy Markdown
Member Author

@ralvarezing the description in the migration guide is not a summary of what your PR does but rather what exactly is the breaking change and what they have to change in their code to migrate. For example: Renamed property from "abc" to "xyz" or removed "abc" use "xyz" instead, or similar.

Yeah!, my bad for doing this and don't read first xD

  • MudRadio, MudCheckBox, MudSwitch: LabelPosition replaced by LabelPlacement and type changes in favor of Placement enum. SwitchLabelClassname renamed to LabelClassName. Checked property replaced by T Value. #9472

@ScarletKuro
Copy link
Copy Markdown
Member

ScarletKuro commented Oct 29, 2024

Tests that still reference the "Placement" property:

  • RadioGroupTest1
  • AllInputsTest

From the RadioContentPlacementExample.razor change, I assume the Placement should be replaced with LabelPlacement, is that correct? If so then it's missing in the migration guide as it talks only about LabelPosition & SwitchLabelClassname renaming.

@ScarletKuro
Copy link
Copy Markdown
Member

Talked to @danielchalmers, he said it was intentionally changed to LabelPlacement fixing in #10135
Will update the migration guide as well

LLauter pushed a commit to cannellamedia/MudBlazor that referenced this pull request Dec 16, 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 (ex: removes/changes a public API)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Radio, Check Box, Switch are not consistent

5 participants