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.

Sign up for GitHub

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

Jump to bottom

fix: don't call binding converter with null values unless only the final segment of the path is null #16114

Merged
merged 26 commits into from May 13, 2024
Merged

fix: don't call binding converter with null values unless only the final segment of the path is null #16114

merged 26 commits into from May 13, 2024

Conversation

ramezgerges
Copy link
Contributor

GitHub Issue (If applicable): closes #16016

PR Type

What kind of change does this PR introduce?

What is the current behavior?

What is the new behavior?

PR Checklist

Please check if your PR fulfills the following requirements:

Other information

Internal Issue (If applicable):

@unodevops
Copy link
Contributor

🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-16114/index.html

Youssef1313
Youssef1313 reviewed Apr 3, 2024
View reviewed changes
Copy link
Member

@Youssef1313 Youssef1313 left a comment

Choose a reason for hiding this comment

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

It's better to write the tests using XAML file so that we can get them validated on WinUI as well.

Also, good to test FallbackValue behavior, and test for both Binding and x:Bind

@unodevops
Copy link
Contributor

🤖 Your WebAssembly Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-16114/index.html

@github-actions github-actions bot added the area/automation Categorizes an issue or PR as relevant to project automation label Apr 5, 2024
@unodevops
Copy link
Contributor

🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-16114/index.html

@unodevops
Copy link
Contributor

🤖 Your WebAssembly Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-16114/index.html

@unodevops
Copy link
Contributor

🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-16114/index.html

1 similar comment
@unodevops
Copy link
Contributor

🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-16114/index.html

@unodevops
Copy link
Contributor

🤖 Your WebAssembly Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-16114/index.html

@unodevops
Copy link
Contributor

🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-16114/index.html

@unodevops
Copy link
Contributor

🤖 Your WebAssembly Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-16114/index.html

@unodevops
Copy link
Contributor

🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-16114/index.html

@unodevops
Copy link
Contributor

🤖 Your WebAssembly Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-16114/index.html

@jeromelaban jeromelaban marked this pull request as draft April 23, 2024 15:50
@jeromelaban
Copy link
Member

@ramezgerges could you take a look? It seems that some tests are not passing.

@Xiaoy312 Xiaoy312 self-assigned this Apr 23, 2024
@Xiaoy312 Xiaoy312 force-pushed the binding_converter_null branch 2 times, most recently from 4c1f666 to 0b3b53d Compare April 24, 2024 20:47
@unodevops
Copy link
Contributor

🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-16114/index.html

1 similar comment
@unodevops
Copy link
Contributor

🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-16114/index.html

@unodevops
Copy link
Contributor

🤖 Your WebAssembly Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-16114/index.html

@ramezgerges ramezgerges marked this pull request as ready for review April 25, 2024 09:47
@unodevops
Copy link
Contributor

🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-16114/index.html

@unodevops
Copy link
Contributor

🤖 Your WebAssembly Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-16114/index.html

@unodevops
Copy link
Contributor

🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-16114/index.html

ramezgerges and others added 24 commits May 9, 2024 13:22
@ramezgerges
chore: formatting
0ad6ff4
@ramezgerges
chore: fix tests
ffe5212
@ramezgerges
test: add test coverage for setting binding context to null in many s…
199bd71 
…cenarios
@ramezgerges
chore: don't use converter when DataContext is null in all cases
fe56901
@ramezgerges
chore: remove BindingPath usage from AutoSuggestBox
c20043d
@ramezgerges
chore: remove IValueChangedListener
70d285a
@ramezgerges
fix(contentpresenter): keep the ContentTemplateRoot if Content is null
ffbe3b9
@ramezgerges
chore: Binding.CompiledSource != null ==> Binding.IsXBind
bb6c045
@ramezgerges
chore: remove BindingFlags
a601cbb
@ramezgerges
fix(xbind): don't change any values when DC is null
e3fc905
@ramezgerges
chore: formatting
66e4487
@ramezgerges
revert: "chore: remove BindingFlags"
ca803ec 
This reverts commit 92e0faf
@ramezgerges
fix: remove TextBox._isTextChangedPending and remove TextBox.IsUserMo…
a60f288 
…difying and send it with the TextChangedEventArgs instead
@ramezgerges
chore: fix AutoSuggestBox regressions after changes
5893253
@ramezgerges
chore: change ContentPresenter_Template to match WinUI behaviour
ef00680
@ramezgerges
chore: fix When_Content_Set_Null_ComboBox to match WinUI and new changes
8d80601
@ramezgerges
chore: fix ItemsControl.CleanUpContainer to align with new changes
8cc2e65
@ramezgerges
chore: fix Given_Binder tests to match WinUI and new changes
c0bb5dd
@ramezgerges
chore: fix tests on iOS, android and wasm
7ef4604
@Xiaoy312 @ramezgerges
chore: fix failing tests
b63003d 
- When_Changes_In_TextChanged // racing condition, needed a WaitForIdle
- When_Text_Changed_Sequence // undo KeyboardHelper.InputText change, that doesnt work
@ramezgerges
chore: adjust When_Text_Changed_Sequence
163f964
@ramezgerges
chore: adjust TextBox_Formatting_FlickerText
82e02e6
@ramezgerges
chore: fix typos in BitmapImage_vs_SvgImageSource tests
df6205b
@ramezgerges
chore: make IsXBind internal
ed04888
@unodevops
Copy link
Contributor

🤖 Your WebAssembly Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-16114/index.html

@unodevops
Copy link
Contributor

🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-16114/index.html

@jeromelaban jeromelaban merged commit b41f493 into unoplatform:master May 13, 2024
103 checks passed
@ramezgerges ramezgerges deleted the binding_converter_null branch May 13, 2024 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Xiaoy312 Xiaoy312 Xiaoy312 left review comments

@Youssef1313 Youssef1313 Youssef1313 left review comments

@jeromelaban jeromelaban jeromelaban approved these changes

Assignees

@Xiaoy312 Xiaoy312

Labels
area/automation Categorizes an issue or PR as relevant to project automation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Converter receiving unexpected null value
5 participants
@ramezgerges @unodevops @jeromelaban @Xiaoy312 @Youssef1313