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

[Core] Add extra method that allows to specify default value for CanExecute on a Command #1177

Closed
wants to merge 2 commits into from

Conversation

rmarinho
Copy link
Member

@rmarinho rmarinho commented Oct 4, 2017

Description of Change

This pr resolves a issue where our CanExecute(obj) method on the Command will always return true if a CanExecute delegate wasn't specified.
The problem happens that in a Button we don't have no way to figure if a user specified or not that delegate on a Command, and we need to decide to either respect the IsEnabled property from the button or the result of the command can execute.

Bugs Fixed

API Changes

Adding to Command
Added:

  • public bool CanExecute(object parameter, bool defaultCanExecute)

Behavioral Changes

None

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Rebased on top of master at time of PR
  • Changes adhere to coding standard
  • Consolidate commits as makes sense

if (cmd != null)
IsEnabledCore = cmd.CanExecute(CommandParameter);
if(cmd != null)
IsEnabledCore = (cmd as Command)?.CanExecute(CommandParameter, IsEnabled) ?? cmd.CanExecute(CommandParameter);
Copy link
Member

Choose a reason for hiding this comment

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

this makes a lot of assumptions

Copy link
Member

@StephaneDelcroix StephaneDelcroix left a comment

Choose a reason for hiding this comment

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

will only work if the ICommand is one of ours... meaning only sometimes, eventually

I'd argue that it's not even a bug. if you set 2 properties fighting to set the IsEnabled value, order do matters.

I wonder how/if UWP handles that...

@rmarinho rmarinho closed this Oct 19, 2017
@rmarinho rmarinho deleted the fix-bugzilla32899 branch December 4, 2017 18:02
@samhouts samhouts modified the milestone: 2.3.0 Jun 27, 2018
mattleibow pushed a commit that referenced this pull request Jan 28, 2021
…ic reference systems

* see issue #1177
* also mention altitude in Location summary
* and fix a typo
mattleibow pushed a commit that referenced this pull request Jan 28, 2021
* Location: add a new member 'AltitudeReferenceSystem'

* and a new enum that represents its type
* in order to deal with platform-specific reference systems
* see issue #1177

* add documentation for Location.AltitudeReferenceSystem

* and the new enum AltitudeReferenceSystem

* Samples: show 'Location.AltitudeReferenceSystem' on GeolocationPage

* add explicit cast for AltitudeReferenceSystem on UWP

* casting from Windows.Devices.Geolocation.AltitudeReferenceSystem to Xamarin.Essentials.AltitudeReferenceSystem

* add a conversion function for AltitudeReferenceSystem on UWP

* in order to make sure that the cast from Windows.Devices.Geolocation.AltitudeReferenceSystem is valid

* Cleanup converstion of ARS for UWP

Co-authored-by: James Montemagno <james.montemagno@gmail.com>
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.

None yet

4 participants