Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MudPicker: Fix code style and naming issues #8517

Merged
merged 4 commits into from
Mar 31, 2024

Conversation

ArieGato
Copy link
Contributor

@ArieGato ArieGato commented Mar 28, 2024

Description

resolves #8489
resolves #8482

  • Add missing Async suffix
  • Fixed naming for Classname properties
  • explicit modifiers
  • removed obsolete code
  • Fix async behavior in HandleKeyDown event

Public API changes

MudColorPicker

  • StringValueChanged -> StringValueChangedAsync

MudBaseDatePicker

  • DateFormatChanged -> DateFormatChangedAsync

MudDatePicker

  • DateFormatChanged -> DateFormatChangedAsync
  • StringValueChanged -> StringValueChangedAsync
  • HandleKeyDown -> OnHandleKeyDownAsync

MudDateRangePicker

  • DateFormatChanged -> DateFormatChangedAsync
  • StringValueChanged -> StringValueChangedAsync

MudPicker

  • PickerClass -> PickerClassname
  • PickerPaperClass -> PickerPaperClassname
  • PickerInlineClass -> 1PickerInlineClassname
  • PickerContainerClass -> PickerContainerClassname
  • PickerInputClass -> PickerInputClassname
  • ActionClass -> ActionsClassname
  • InputIcon (removed obsolete) -> AdornmentIcon
  • InputVariant (removed obsolete) -> Variant
  • AllowKeyboardInput (removed, unused)
  • ClassActions -> ActionsClass
  • StringValueChange -> StringValueChangedAsync
  • HandleKeyDown -> OnHandleKeyDownAsync

MudPickerContent

  • Classnames -> Classname

MudPickerToolbar

  • Classnames -> Classname

MudTimePicker

  • StringValueChanged -> StringValueChangedAsync
  • HandleKeyDown -> OnHandleKeyDownAsync

How Has This Been Tested?

Tested the Server Docs pages
Unit tests

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)

Checklist:

  • The PR is submitted to the correct branch (dev).
  • My code follows the code style of this project.
  • I've added relevant tests.

- Async postfix
- Fixed naming for Classname properties
- explicit modifiers
- removed obsolete code
Fix async behavior in HandleKeyDown event
Copy link

codecov bot commented Mar 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.30%. Comparing base (6c6f610) to head (d811a99).
Report is 3 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #8517      +/-   ##
==========================================
+ Coverage   89.29%   89.30%   +0.01%     
==========================================
  Files         411      413       +2     
  Lines       11901    11905       +4     
  Branches     2356     2357       +1     
==========================================
+ Hits        10627    10632       +5     
  Misses        751      751              
+ Partials      523      522       -1     

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

@ScarletKuro ScarletKuro added API change API that needs approval v7 New major MudBlazor version refactor Refactoring of code. No bug fixes or new features labels Mar 29, 2024
@ScarletKuro
Copy link
Member

I edited your post with all public API renames if you don't mind.

@ScarletKuro ScarletKuro requested a review from henon March 29, 2024 01:07
@ScarletKuro
Copy link
Member

LGTM, just need to fix the formatting in one place.

And thanks for the xmldocs, it was bothering me for a very long time

@danielchalmers
Copy link
Contributor

Also resolves #8482

@ArieGato
Copy link
Contributor Author

Also resolves #8482

It looks like a dup.

@ArieGato
Copy link
Contributor Author

All done

@ArieGato
Copy link
Contributor Author

@ScarletKuro @henon is there something that I need to do for this pr?

@ScarletKuro
Copy link
Member

No, I'm waiting when @henon will review it.

@henon
Copy link
Collaborator

henon commented Mar 31, 2024

OK, just this #8517 (comment) left to be done then this can be merged

@henon henon mentioned this pull request Mar 31, 2024
@ArieGato
Copy link
Contributor Author

OK, just this #8517 (comment) left to be done then this can be merged

All done, I did it already.

@henon henon merged commit b09514f into MudBlazor:dev Mar 31, 2024
4 checks passed
@henon
Copy link
Collaborator

henon commented Mar 31, 2024

Added to v7 Migration Guide #8447

@henon
Copy link
Collaborator

henon commented Mar 31, 2024

Thanks @ArieGato

@ArieGato ArieGato deleted the feature/8489-mudpicker-naming branch April 1, 2024 08:11

<MudColorPicker Label="Basic Color Picker" @bind-Text="_colorValue" Style="@($"color: {_colorValue};")" Placeholder="Select Color" />

@code {
private string _colorValue;
private string _colorValue = "#000000";
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi, was this changed because of the MudColor constructor exception? I just fixed that so do you mind if I change this back in my PR? Hard to see now on dark theme.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It threw a null ref exception.

danielchalmers added a commit to danielchalmers/MudBlazor that referenced this pull request Apr 7, 2024
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
API change API that needs approval breaking change refactor Refactoring of code. No bug fixes or new features v7 New major MudBlazor version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Picker: Rename ClassActions to ActionsClass v7 Rename ClassActions parameter in MudPicker
4 participants