microsoft / microsoft-ui-xaml

Windows UI Library: the latest Windows 10 native controls and Fluent styles for your applications
MIT License
6.35k stars 677 forks source link

[Feature] x:Bind should support general string indexers without interface in C# #3064

Open michael-hawker opened 4 years ago

michael-hawker commented 4 years ago

Describe the bug The documentation for x:Bind here in the 'path to the function' section specifies:

The path to the function is specified like other property paths and can include dots (.), indexers or casts to locate the function.

However, it appears that non-integer based indexers, e.g. strings, are not supported?

I was trying to bind directly to a Graph function with the Windows Community Toolkit's GraphPresenter:

RequestBuilder="{x:Bind providers:ProviderManager.Instance.GlobalProvider.Graph.Teams['<guid here>'].Channels['<guid thing here>'].Messages, Mode=OneWay}"

As these classes use string indexers to build the query to the Graph, E.g.

ITeamRequestBuilder this[string id] { get; }

However VS tells me in the x:Bind expression that I have provided an "invalid index value" and "WMC1110: Invalid binding path ... Unexpected array indexer".

Expected behavior I would have expected string indexers to work just as I can pass them into function parameters with x:Bind in single-quotes.

Version Info NuGet package version: WUX

Windows 10 version Saw the problem?
Insider Build (xxxxx)
May 2020 Update (19041) Yes
November 2019 Update (18363)
May 2019 Update (18362)
October 2018 Update (17763)
April 2018 Update (17134)
Fall Creators Update (16299)
Creators Update (15063)
Device form factor Saw the problem?
Desktop Yes
Xbox
Surface Hub
IoT
michael-hawker commented 4 years ago

FYI @oldnewthing even showed a string indexer as an example in his article here in the opening paragraph:

<TextBlock Text=”{x:Bind Players[‘John Smith’]” />

Thanks @chingucoding for the article link! 🦙❤

Felix-Dev commented 4 years ago

@michael-hawker As he quoted from the documentation, here's the official part: https://docs.microsoft.com/en-us/windows/uwp/xaml-platform/x-bind-markup-extension#collections

michael-hawker commented 4 years ago

Thanks @Felix-Dev, @chingucoding just pointed me to that doc as well. I think the "functions in x:Bind doc" should link to the main article, I'll open up a doc issue about that.

So it seems like this is a feature request then as this is currently expected behavior as documented. Just not looking at the right doc... 😋

oldnewthing commented 4 years ago

As noted in the documentation, the collection must be mutable. But your indexer is read-only.

(Note that IReadOnlyDictionary<T> and IMapView<T> do not support the indexer syntax.)

michael-hawker commented 4 years ago

@StephenLPeters I understand that this was not intended as designed originally, but this is still a feature request.

Is there a reason the collection needs to be mutable to support this feature? Can we get a few more details here? I know it'll still need WinUI 3 to be fixed most likely in the future, if possible.

Thanks!

StephenLPeters commented 4 years ago

@StephenLPeters I understand that this was not intended as designed originally, but this is still a feature request.

Is there a reason the collection needs to be mutable to support this feature? Can we get a few more details here? I know it'll still need WinUI 3 to be fixed most likely in the future, if possible.

Thanks!

Sorry the triage team interpretated the conversation differently. We can reuse this as a feature proposal

michael-hawker commented 3 years ago

@StephenLPeters can we get this re-opened as a feature request for the future? Thanks!

JeanRoca commented 3 years ago

Hey @michael-hawker this is something that would need to come in WinUI 3, but it is not currently something we are currently looking at. Would you be able to go into more detail on what scenarios you are trying to support here so our team can re-evaluate this?

michael-hawker commented 3 years ago

@JeanRoca the scenario we had came directly from us trying to make it easier to integrate our XAML presenter with the Graph SDK. The Graph SDK is just .NET Standard based, so it's using base interface/types that we wouldn't be able to modify.

It'd be great if x:Bind could just use the indexer overload if one exists on whatever object it's dealing with.

JeanRoca commented 3 years ago

Alright I see thank you for the insight. I will go ahead and place it in the backlog, but can't quite guarantee an ETA for it yet. I will leave the issue open.

michael-hawker commented 3 years ago

@JeanRoca @StephenLPeters @oldnewthing does this restriction apply to regular bindings and VisualStates as well?

@ismaelestalayo was just trying to change DataGrid.Columns with indexing with the standard syntax as well, and couldn't get it to work. Looks like DataGrid.Columns was defined directly as an ObservableCollection vs. an IList. (FYI @anawishnoff @RBrid)

<VisualState x:Name="NarrowState">
    <VisualState.StateTriggers>
        <AdaptiveTrigger MinWindowWidth="0" />
    </VisualState.StateTriggers>
    <VisualState.Setters>
        <Setter Target="(DataGrid.Columns)[0].Width" Value="0"/>
        <Setter Target="(DataGrid.Columns[1]).Width" Value="0"/>
        <Setter Target="(DataGrid.Columns.DataGridTemplateColumn[2]).Width" Value="0"/>
    </VisualState.Setters>
</VisualState>
cesarchefinho commented 2 years ago

I solve this bug with an xaml extension to bind XAML to an string index property without c# interface

see https://github.com/CommunityToolkit/Labs-Windows/issues/236

If microsoft aprove the spec i will submit the code

RedTellYou commented 2 weeks ago

@JeanRoca I have a requirement for reference. Since the project needs to provide both WPF and WinUI3 versions, the multi-language design needs to support both WPF and WinUI3, and needs to support dynamic language switching. So I hope you can support the function of universal string indexing. (#10033 )