Skip to content

Rename remaining Disable... properties#8826

Merged
henon merged 12 commits intoMudBlazor:devfrom
henon:color-picker-renames
Apr 28, 2024
Merged

Rename remaining Disable... properties#8826
henon merged 12 commits intoMudBlazor:devfrom
henon:color-picker-renames

Conversation

@henon
Copy link
Contributor

@henon henon commented Apr 27, 2024 •
edited
Loading

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.

@github-actions github-actions bot added breaking change This change will require consumer code updates PR: needs review labels Apr 27, 2024
@henon
Copy link
Contributor Author

henon commented Apr 27, 2024

FYI @BieleckiLtd

@henon henon added API change Modifies the public API surface v7 labels Apr 27, 2024
ScarletKuro
ScarletKuro reviewed Apr 27, 2024
View reviewed changes
@codecov
Copy link

codecov bot commented Apr 27, 2024 •
edited
Loading

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.

jperson2000
jperson2000 approved these changes Apr 27, 2024
View reviewed changes
Copy link
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
Member

danielchalmers commented Apr 27, 2024 •
edited
Loading

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
Contributor 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.

henon added 2 commits April 27, 2024 22:05
@henon
fix + update
4cc48fb
@henon
Merge remote-tracking branch '_MudBlazor/dev' into color-picker-renames
4bbd194 
# Conflicts:
#	src/MudBlazor/Components/Chart/Models/ChartOptions.cs
@henon
Copy link
Contributor 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
Contributor Author

henon commented Apr 27, 2024

Ready for merge.

@jperson2000 jperson2000 mentioned this pull request Apr 28, 2024
Merged
7 tasks
@henon
Merge remote-tracking branch '_MudBlazor/dev' into color-picker-renames
66c31a4 
# Conflicts:
#	src/MudBlazor/Base/MudComponentBase.cs
@henon henon merged commit 5a505b0 into MudBlazor:dev Apr 28, 2024
@henon henon deleted the color-picker-renames branch April 28, 2024 11:49
@henon henon mentioned this pull request Apr 28, 2024
Closed
@henon
Copy link
Contributor Author

henon commented Apr 28, 2024

Added to v7.0.0 Migration Guide #8447

henon added a commit that referenced this pull request May 4, 2024
@henon
MudColorPicker: Revert ShowToolbar default value to false (#8826)
f98e9a1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ScarletKuro ScarletKuro ScarletKuro left review comments

@danielchalmers danielchalmers Awaiting requested review from danielchalmers

+1 more reviewer

@jperson2000 jperson2000 jperson2000 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

API change Modifies the public API surface breaking change This change will require consumer code updates

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Negative property names should be discouraged

4 participants

@henon @danielchalmers @jperson2000 @ScarletKuro