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

[Proposal] EventArgsConverter #1134

Closed
5 of 8 tasks
dnviti opened this issue Apr 7, 2023 · 3 comments · Fixed by #1219
Closed
5 of 8 tasks

[Proposal] EventArgsConverter #1134

dnviti opened this issue Apr 7, 2023 · 3 comments · Fixed by #1219
Labels
blocked new proposal A fully fleshed out proposal describing a new feature in syntactic and semantic detail

Comments

@dnviti
Copy link

dnviti commented Apr 7, 2023

Feature name

EventArgsConverter

Link to discussion

#1133

Progress tracker

  • Android Implementation
  • iOS Implementation
  • MacCatalyst Implementation
  • Windows Implementation
  • Tizen Implementation
  • Unit Tests
  • Samples
  • Documentation

Summary

Should create a new EventArgsConverter to be used to catch generic EventArgs from any EventToCommandBehavior event and be able to pass it to the references Command.

Motivation

To be able to catch any EventArgs and pass to a Command

Detailed Design

<ContentPage.Resources>
    <ResourceDictionary>
        <convs:EventArgsConverter x:Key="EventArgsConverter" />
    </ResourceDictionary>
</ContentPage.Resources>
...
<toolkit:EventToCommandBehavior
EventName="SelectionChanged"
EventArgsConverter="{StaticResource Key=EventArgsConverter}"
Command="{Binding SelectionChangedCommand}" />

And it's Command Usage

new Command<AnyEventArgsYouNeed>(async (args) => await Task.Delay(1));

Usage Syntax

Should create a new EventArgsConverter to be used to catch generic EventArgs from any EventToCommandBehavior event and be able to pass it to the references Command like so:


<ContentPage.Resources>
    <ResourceDictionary>
        <convs:EventArgsConverter x:Key="EventArgsConverter" />
    </ResourceDictionary>
</ContentPage.Resources>
...
<toolkit:EventToCommandBehavior
EventName="SelectionChanged"
EventArgsConverter="{StaticResource Key=EventArgsConverter}"
Command="{Binding SelectionChangedCommand}" />


And it's Command Usage

```cs
new Command<AnyEventArgsYouNeed>(async (args) => await Task.Delay(1));


### Drawbacks

_No response_

### Alternatives

_No response_

### Unresolved Questions

_No response_
@dnviti dnviti added new proposal A fully fleshed out proposal describing a new feature in syntactic and semantic detail labels Apr 7, 2023
@brminnick
Copy link
Collaborator

brminnick commented Apr 7, 2023

Hi @dnviti! I've added the blocked tag until we've had a discussion on this new feature.

It looks like you've skipped a few steps and opened a proposal before we've iterated over its API design in its Discussion. You can find detailed information on how to add new features to CommunityToolkit.Maui in the Submitting A Feature sectionin our README.

I'd like to see the actual API Design. You can see previous Converter Proposals for examples on how to document an API design.

I'd also like to see a real example that implements and uses this feature, showing a real use-case. The current example just demonstrates a Command calling Task.Delay and I assume our users will not be writing code like this when implementing this feature.

Let's please move back to the Discussion to design and finalize the API and to understand the motivation and intent behind this new feature.

@brminnick brminnick changed the title [Proposal] Add an EventArgsConverter to catch generic Events [Proposal] EventArgsConverter Apr 7, 2023
@VladislavAntonyuk VladislavAntonyuk added the needs discussion Discuss it on the next Monthly standup label May 9, 2023
@bijington
Copy link
Contributor

I can see from the code that our EventToCommandBehavior<TType> does pass in the event args in as the parameter to the Command if noEventArgsConverter or CommandParameter are provided. This does currently rely on the need to define the type of the event args.

Is the aim of this proposal to prevent needing to define the type (e.g. using x:TypeArguments="TYPE") in XAML? And have it simply resort to providing EventArgs in? If so then perhaps the cleaner change would be to modify the EventToCommandBehavior class (note the lack of <TType>) to pass in the event args if noEventArgsConverter or CommandParameter are provided as per the generic implementation?

@ghost
Copy link

ghost commented Jun 13, 2023

Reopening Proposal.

Only Proposals moved to the Closed Project Column and Completed Project Column can be closed.

@ghost ghost reopened this Jun 13, 2023
@ghost ghost closed this as completed Jun 13, 2023
@brminnick brminnick removed the needs discussion Discuss it on the next Monthly standup label Sep 7, 2023
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked new proposal A fully fleshed out proposal describing a new feature in syntactic and semantic detail
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants