-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
IsEditable combox box #18094
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
base: master
Are you sure you want to change the base?
IsEditable combox box #18094
Conversation
This sounds very similar to the autocomplete box? Is it different somehow? https://docs.avaloniaui.net/docs/reference/controls/autocompletebox |
Please read the following Contributor License Agreement (CLA). If you agree with the CLA, please reply with the following:
Contributor License AgreementContribution License AgreementThis Contribution License Agreement ( “Agreement” ) is agreed to by the party signing below ( “You” ), 1. Definitions. “Code” means the computer software code, whether in human-readable or machine-executable form, “Project” means any of the projects owned or managed by AvaloniaUI OÜ and offered under a license “Submit” is the act of uploading, submitting, transmitting, or distributing code or other content to any “Submission” means the Code and any other copyrightable material Submitted by You, including any 2. Your Submission. You must agree to the terms of this Agreement before making a Submission to any 3. Originality of Work. You represent that each of Your Submissions is entirely Your 4. Your Employer. References to “employer” in this Agreement include Your employer or anyone else 5. Licenses. a. Copyright License. You grant AvaloniaUI OÜ, and those who receive the Submission directly b. Patent License. You grant AvaloniaUI OÜ, and those who receive the Submission directly or c. Other Rights Reserved. Each party reserves all rights not expressly granted in this Agreement. 6. Representations and Warranties. You represent that You are legally entitled to grant the above 7. Notice to AvaloniaUI OÜ. You agree to notify AvaloniaUI OÜ in writing of any facts or 8. Information about Submissions. You agree that contributions to Projects and information about 9. Governing Law/Jurisdiction. This Agreement is governed by the laws of the Republic of Estonia, and 10. Entire Agreement/Assignment. This Agreement is the entire agreement between the parties, and AvaloniaUI OÜ dedicates this Contribution License Agreement to the public domain according to the Creative Commons CC0 1. |
In 2 ways that I can see:
It's also a much more familiar experience for people coming from WPF that I think it's worth the small change |
@cla-avalonia agree |
Public API for review: namespace Avalonia.Controls
{
public class ComboBox : SelectingItemsControl
{
+ public static readonly DirectProperty<ComboBox, bool> IsEditableProperty;
+ public static readonly StyledProperty<string?> TextProperty;
+ public static readonly StyledProperty<IBinding?> ItemTextBindingProperty;
+ public bool IsEditable { get; set; }
+ public string? Text { get; set; }
+ public IBinding? ItemTextBinding { get; set; }
}
} |
During the API review meeting, we noticed that WPF uses Remove:
Add:
Also, (Not API related, but |
I wasn't aware of I don't see a way to do that using the attached property since I can't access the |
To clarify, I've implemented it in #18405 so this PR can focus on the implementation of |
Ahh, sorry I miss read, my brain didn't connect that Appreciate the PR, will update my branch once it's merged in |
@MrJul thanks for your help with this 😄 I've updated it to work with what you've done and put some tests in (and fixed broken ones). Also made a docs pull request |
You can test this PR using the following package version. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have left some of my thoughts. But I am not in charge around here, so someone may disagree.
TextSearch.TextBinding="{Binding SearchText, DataType=viewModels:IdAndName}" | ||
SelectedItem="{Binding SelectedItem}" /> | ||
|
||
<TextBlock Text="Editable text is bound to SearchText. Display is bound to Name" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should use full property names to make this clearer. Not everyone will go straight to XAML when running the catalogue.
@@ -17,6 +17,20 @@ public bool WrapSelection | |||
set => this.RaiseAndSetIfChanged(ref _wrapSelection, value); | |||
} | |||
|
|||
private string _textValue = string.Empty; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In C#, variables generally go near the top of the class and are grouped together.
TextSearch.TextBindingProperty.Changed.AddClassHandler<ComboBox>((x, e) => x.ItemTextBindingChanged(e)); | ||
//when the items change we need to simulate a text change to validate the text being an item or not and selecting it | ||
ItemsSourceProperty.Changed.AddClassHandler<ComboBox>((x, e) => x.TextChanged( | ||
new AvaloniaPropertyChangedEventArgs<string?>(e.Sender, TextProperty, x.Text, x.Text, e.Priority))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might be an issue if something is listening for text change events? It might not. I have not tested it. Personally though, I would expect a text change event to only ever be fired when the text actually changes.
@@ -188,6 +221,21 @@ public IDataTemplate? SelectionBoxItemTemplate | |||
set => SetValue(SelectionBoxItemTemplateProperty, value); | |||
} | |||
|
|||
/// <summary> | |||
/// Gets or sets the text used when <see cref="IsEditable"/> is true. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should document what this does if IsEditable
is not true.
@@ -315,6 +363,15 @@ protected override void OnPointerPressed(PointerPressedEventArgs e) | |||
/// <inheritdoc/> | |||
protected override void OnPointerReleased(PointerReleasedEventArgs e) | |||
{ | |||
//if the user clicked in the input text we don't want to open the dropdown |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does WPF/winforms do this? I seem to vaguely recall that it actually opens the drop down and filters the list as you type. But it has been a long time, so I could be misremembering.
… have text binding properties
You can test this PR using the following package version. |
What does the pull request do?
Adds IsEditable property to ComboBox and adds a ItemTextBinding property to get the text value.
What is the current behavior?
Currently it's unsupported.
What is the updated/expected behavior with this PR?
To be able to bind text values in a combo box and it handle the text values and SelectedItem values
How was the solution implemented (if it's not obvious)?
Added a text box which is bound to TextProperty and only visible when IsEditable is true. Compares the value of the text box with the ItemsSource via an IBinding (similar to the auto complete box)
Checklist
Breaking changes
None
Fixed issues
#205.
I am aware that pull request #1070 but it looks like a lot of "ToString" stuff was going on which doesn't really fit with Avalonia, hoping this way is more acceptable