Skip to content

Multipbinding support for Loc#273

Open
konne wants to merge 4 commits intodevelopmentfrom
feature/loc-multibinding-support
Open

Multipbinding support for Loc#273
konne wants to merge 4 commits intodevelopmentfrom
feature/loc-multibinding-support

Conversation

@konne
Copy link
Copy Markdown
Member

@konne konne commented Aug 17, 2020

This small change have a small breaking change and for me a huge benefit:

breaks:
<lex:Loc>keyName</lex:Loc>

feature:

 <TextBlock>
        <TextBlock.Text>
            <lex:Loc>
                <MultiBinding Converter="...">
                    <Binding Path="PATH" "/>
                </MultiBinding>
            </lex:Loc>
        </TextBlock.Text>
</TextBlock>

@konne konne requested a review from SeriousM August 17, 2020 13:01
@konne
Copy link
Copy Markdown
Member Author

konne commented Aug 17, 2020

@Karnah @SeriousM @JeremyWu917 @benjaminrupp @
Feedback is highly welcome, because of the breaking change.
Be honest I see nearly null impact, but please give me your feedback.

@SeriousM
Copy link
Copy Markdown
Collaborator

Does that mean that <lex:Loc>keyName</lex:Loc> is not possible anymore? I would welcome that the simplest binding expression is still available as it's the lowest barrier to use the extension. Can't you check if ResourceIdentifierKey is a string or BindingBase instead of forcing it to be BindingBase?

@konne
Copy link
Copy Markdown
Member Author

konne commented Aug 17, 2020

@SeriousM
I think nearly nobody is using this approach, that will break

 <TextBlock>
        <TextBlock.Text>
            <lex:Loc>KEY</lex:Loc>
        </TextBlock.Text>
</TextBlock

I think everybody is using this approach and this will still work.

 <TextBlock Text={lex:Loc KEY} />

@SeriousM
Copy link
Copy Markdown
Collaborator

I think everybody is using this approach and this will still work.

Ah I see, that's all a newcomer needs and is even more friendly.

@Karnah
Copy link
Copy Markdown
Contributor

Karnah commented Aug 17, 2020

LGTM!

Can't you check if ResourceIdentifierKey is a string or BindingBase instead of forcing it to be BindingBase?

I think it allows to avoid breaking changes. Or do you want to make LocBinding more simple?

Comment thread tests/HelloWorldWPF/MainWindow.xaml Outdated
Comment on lines +80 to +82
<MultiBinding StringFormat="{}{0}">
<Binding Path="tenum" StringFormat="TestEnum_{0}"/>
</MultiBinding>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It is not work as expected. I think it shold be like that:

<MultiBinding StringFormat="{}TestEnum_{0}">
    <Binding Path="tenum" />
</MultiBinding>

Explanation
And yes, my version looks weird :)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It is not work as expected. I think it shold be like that:

<MultiBinding StringFormat="{}TestEnum_{0}">
    <Binding Path="tenum" />
</MultiBinding>

Explanation
And yes, my version looks weird :)

@Karnah
I change the example to avoid this MultiBinding StringFormat Bug. Thanks for the Explanation.

@konne
Copy link
Copy Markdown
Member Author

konne commented Aug 18, 2020

I think it allows to avoid breaking changes. Or do you want to make LocBinding more simple?

@Karnah in the future I like to deprecate LocBinding. I like if we have in the end nearly one extension Loc that solves all problems.

@konne konne self-assigned this Aug 18, 2020
@konne konne added this to the 4.0 milestone Aug 18, 2020
@konne
Copy link
Copy Markdown
Member Author

konne commented Aug 18, 2020

Can't you check if ResourceIdentifierKey is a string or BindingBase instead of forcing it to be BindingBase?
I don't found a way. Ideas are highly welcome.

@Karnah
Copy link
Copy Markdown
Contributor

Karnah commented Aug 18, 2020

I don't found a way. Ideas are highly welcome.

@konne. yes, you fully right. Looks like it's not possible to use object type for MultiBinding. I have found another way, which allows avoid breaking changes, but it can look a bit ugly. It is necessary to change ContentProperty to Key and use MultiBinding for ResourceIdentifierKey property.

<TextBlock>
    <TextBlock.Text>
        <lex:Loc>
            <lex:Loc.ResourceIdentifierKey>
                <MultiBinding StringFormat="{}TestEnum_{0}">
                    <Binding Path="tenum" />
                </MultiBinding>
            </lex:Loc.ResourceIdentifierKey>
        </lex:Loc>
    </TextBlock.Text>
</TextBlock>

<TextBlock>
    <TextBlock.Text>
        <lex:Loc>
            TestEnum_Test1
        </lex:Loc>
    </TextBlock.Text>
</TextBlock>

@konne konne removed this from the 4.0 milestone Aug 18, 2020
@konne
Copy link
Copy Markdown
Member Author

konne commented Aug 18, 2020

@Karnah be honest I always happy to get your feedback and input.
I combined your finding and I think I will post tomorrow a good solution, described below:

For now you can already access the new Binding in the current NameSpace (http://wpflocalizeextension.codeplex.com) with

<TextBlock>
    <TextBlock.Text>
        <lex:Loc>
            <lex:Loc.KeyBinding>
                <MultiBinding StringFormat="{}TestEnum_{0}">
                    <Binding Path="tenum" />
                </MultiBinding>
            </lex:Loc.KeyBinding>
        </lex:Loc>
    </TextBlock.Text>
</TextBlock>

but If you switch to the new namespace that I have introduced you can already use the new syntax that will be introduced in 4.0 (also with all deprecation).
But if you like you can to it even xaml file by xaml file / step by step.

@Karnah
Copy link
Copy Markdown
Contributor

Karnah commented Aug 20, 2020

I think your solution is really excellent. Unfortunately, I cannot update package in our projects now. But when I have more time, I will do it.
Thank you for all

@ciuckc
Copy link
Copy Markdown

ciuckc commented Sep 24, 2024

Hello, any news about this feature?

@SeriousM
Copy link
Copy Markdown
Collaborator

@konne, can you fix the conflict somehow? it's a very old change and I really don't remember the details anymore nor do I used wpf for years. sorry @ciuckc for that...

@ciuckc
Copy link
Copy Markdown

ciuckc commented Nov 5, 2024

@konne, can you fix the conflict somehow? it's a very old change and I really don't remember the details anymore nor do I used wpf for years. sorry @ciuckc for that...

Ah no worries thank you for you response also!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants