Skip to content
This repository was archived by the owner on May 1, 2024. It is now read-only.

Add BaseCommand.ChangeCanExecute() #736

Merged
merged 5 commits into from
Jan 14, 2021

Conversation

TheCodeTraveler
Copy link
Contributor

This PR is base on community feedback from @maxkoshevoi: #709 (comment)

Description of Change

Because Xamarin.Forms.Command uses ChangeCanExecute(), adding ChangeCanExecute() to both AsyncCommand and AsyncValueCommand enables an easier, more seamless, transition from Xamarin.Forms.Command to Xamarin.CommunityToolkit.ObjectModel.AsyncCommand.

API Changes

Adds BaseCommand.ChangeCanExecute()

  • BaseCommand.ChangeCanExecute() leverages BaseCommand.RaiseCanExecuteChanged()

Behavioral Changes

None, BaseCommand.ChangeCanExecute() leverages BaseCommand.RaiseCanExecuteChanged():

public void ChangeCanExecute() => RaiseCanExecuteChanged();

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Has samples (if omitted, state reason in description)
  • Rebased on top of main at time of PR
  • Changes adhere to coding standard
  • Updated documentation

This encourages developers to use RaiseCanExecuteChanged to avoid maintaing and documenting two methods that perform the same functionality
@TheCodeTraveler
Copy link
Contributor Author

Assigning to @jfversluis for review since he was looped into the original discussion: #709 (comment)

@AndreiMisiukevich AndreiMisiukevich self-requested a review January 11, 2021 00:51
@TheCodeTraveler TheCodeTraveler removed the request for review from jfversluis January 12, 2021 18:33
@pictos
Copy link
Contributor

pictos commented Jan 12, 2021

@brminnick since this PR makes an API Obsolete I think that is an API change, so will be better to target the develop branch. Thoughts?

@TheCodeTraveler
Copy link
Contributor Author

TheCodeTraveler commented Jan 12, 2021

@pictos It doesn't make an API obsolete. It adds an API, improving the migration experience from Xamarin.Forms.Command to AsyncCommand.

Users of AsyncCommand should continue using RaiseCanExecuteChanged. It is the recommended API in the docs and the API used in the samples.

This PR adds ChangeCanExecute() to allow for an easier migration from Xamarin.Forms.Command.

I originally attributed ChangeCanExecute with the [Obsolete] tag so that intellisense would nudge the user to use RaiseCanExecuteChanged without breaking their implementation of Xamarin.Forms.Command. It makes for a easy and pleasant developer experience to have code that still runs but to also have intellisense provide the recommended usage.

As a compromise to not adding [Obsolete] to the code base, I changed it to [EditorBrowsable(EditorBrowsableState.Never)]. This provides a similar migration experience, but without additional benefit of recommending the recommended API that the [Obsolete] attribute provides.

@pictos
Copy link
Contributor

pictos commented Jan 12, 2021

@brminnick my mistake so, thanks for explaining that. Yeah, I would say that we are good to go then.

@TheCodeTraveler TheCodeTraveler dismissed AndreiMisiukevich’s stale review January 12, 2021 23:46

Changed [Obsolete] to [EditorBrowsable]

@TheCodeTraveler TheCodeTraveler enabled auto-merge (squash) January 12, 2021 23:46
@pictos pictos disabled auto-merge January 14, 2021 02:50
@pictos pictos merged commit 04c3cd5 into xamarin:main Jan 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants