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

Rename remaining Disable... properties #8826

Merged
merged 12 commits into from
Apr 28, 2024
Merged

Conversation

henon
Copy link
Collaborator

@henon henon commented Apr 27, 2024

Description

Resolves #6131

  • ChartOptions: replace DisableLegend with ShowLegend and invert value
  • MudPickerToolbar: replace DisableToolbar with ShowToolbar and invert value
  • MudPicker: replace DisableToolbar with ShowToolbar and invert value
  • MudPicker: replace ToolBarClass with ToolbarClass
  • MudDialog: replace DisableSidePadding with Gutters and invert value
  • MudDialog: replace DisableOverlay with Overlay and invert value
  • MudTabs: replace DisableSliderAnimation with SliderAnimation and invert value
  • MudTimeline: replace DisableModifiers with Modifiers and invert value

MudColorPicker

  • Replace DisableSliders with ShowSliders and invert value
  • Replace DisablePreview with ShowPreview and invert value
  • Replace DisableModeSwitch with ShowModeSwitch and invert value
  • Replace DisableInputs with ShowInputs and invert value
  • Replace DisableDragEffect with DragEffect and invert value
  • Replace DisableColorField with ShowColorField and invert value
  • Replace DisableAlpha with ShowAlpha and invert value

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.

@henon
Copy link
Collaborator Author

henon commented Apr 27, 2024

FYI @BieleckiLtd

@henon henon added API change API that needs approval v7 New major MudBlazor version labels Apr 27, 2024
Copy link

codecov bot commented Apr 27, 2024

Codecov Report

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

Project coverage is 90.12%. Comparing base (28bc599) to head (66c31a4).
Report is 125 commits behind head on dev.

Files Patch % Lines
...lazor/Components/Dialog/MudDialogInstance.razor.cs 33.33% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #8826      +/-   ##
==========================================
+ Coverage   89.82%   90.12%   +0.29%     
==========================================
  Files         412      421       +9     
  Lines       11878    12216     +338     
  Branches     2364     2410      +46     
==========================================
+ Hits        10670    11010     +340     
+ Misses        681      664      -17     
- Partials      527      542      +15     

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

Copy link
Sponsor Contributor

@jperson2000 jperson2000 left a comment

Choose a reason for hiding this comment

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

The code is so much more intuitive after these renames!

@danielchalmers
Copy link
Contributor

danielchalmers commented Apr 27, 2024

Some are ambiguous or hard to tell what it does at a glance. The problem has always been that it's easy for these properties to sound like configuration points (Alpha sounds like you can change the alpha).

Some ideas:

  • SliderAnimation -> AnimateSlider
  • Preview -> ShowPreview
  • ModeSwitch -> AllowSwitchingModes
  • DragEffect -> AllowPointerDrag (i would remove this property entirely)
  • ColorField -> probably fine but also: ShowColorField (ShowColorSpectrum??)
  • Alpha -> EnableAlpha??
  • Toolbar makes sense but at the same time we have a ToolBar component so I would favor consistency

@henon
Copy link
Collaborator Author

henon commented Apr 27, 2024

I like AnimateSlider and ShowPreview.
As for Toolbar, the component is called MudPickerToolbar. I guess we really should rename MudToolBar.

# Conflicts:
#	src/MudBlazor/Components/Chart/Models/ChartOptions.cs
@henon
Copy link
Collaborator Author

henon commented Apr 27, 2024

I decided to keep SliderAnimation. It is consistent with other things like Ripple. DragEffect is also consistent with that.

In ColorPicker I added Show almost everywhere, it feels better that way and is more consistent also: ShowPreview, ShowColorField, ShowAlpha, ShowModeSwitch.

@henon
Copy link
Collaborator Author

henon commented Apr 27, 2024

Ready for merge.

# Conflicts:
#	src/MudBlazor/Base/MudComponentBase.cs
@henon henon merged commit 5a505b0 into MudBlazor:dev Apr 28, 2024
4 checks passed
@henon henon deleted the color-picker-renames branch April 28, 2024 11:49
@henon henon mentioned this pull request Apr 28, 2024
@henon
Copy link
Collaborator Author

henon commented Apr 28, 2024

Added to v7.0.0 Migration Guide #8447

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 v7 New major MudBlazor version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Negative property names should be discouraged
4 participants