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

Search View #380

Merged
merged 44 commits into from
Dec 1, 2021
Merged

Search View #380

merged 44 commits into from
Dec 1, 2021

Conversation

nCastle1
Copy link
Contributor

@nCastle1 nCastle1 commented Oct 6, 2021

Putting this up now to centralize discussion and get early feedback.

Updated OMD (2021-11-25):

image

image

image

@nCastle1 nCastle1 changed the title Early draft: Search Search View Nov 18, 2021
@nCastle1 nCastle1 marked this pull request as ready for review November 18, 2021 01:17
@nCastle1 nCastle1 mentioned this pull request Nov 22, 2021
public partial class SearchView : TemplatedView, INotifyPropertyChanged
{
#pragma warning disable SA1306, SA1310, SX1309 // Field names should begin with lower-case letter
private Entry? PART_Entry;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There were named on the assumption that there would be a TemplatePartAttribute equivalent for Xamarin.Forms, but there doesn't appear to be. Am I missing something?

Visibility="{Binding SearchViewModel.Results.Count, Mode=OneWay, Converter={StaticResource CollectionIsEmptyToBoolConverter}, ConverterParameter='Empty'}">
<Grid>
<ListView
ItemTemplate="{StaticResource SuggestionTemplate}"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was conflicted about how many customization properties it made sense to provide. It would be trivial to add properties for SuggestionTemplate, ResultTemplate, SourceTemplate, etc, but then would it also make sense to SuggestionListStyle, SuggestionListItemContainerStyle, and so on? The UI is complicated enough that it may not make sense to change part without changing the whole thing.

private ISearchSource? _activeSource;
private SearchResult? _selectedResult;
private string? _currentQuery;
private string? _defaultPlaceholder = "Find a place or address";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should support for localization with resx be included in new controls as MVP?

Copy link
Member

@dotMorten dotMorten Nov 29, 2021

Choose a reason for hiding this comment

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

Yes we already have a resx file you can use https://github.com/Esri/arcgis-toolkit-dotnet/blob/main/src/Toolkit/Toolkit/LocalizedStrings/Resources.resx
This should however be a property on the control, as it's a visual thing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've filed an issue to track this work: #386

Copy link
Member

@dotMorten dotMorten left a comment

Choose a reason for hiding this comment

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

Just fix typo

@nCastle1 nCastle1 requested a review from dotMorten November 30, 2021 21:15
@nCastle1 nCastle1 merged commit d87ee8b into main Dec 1, 2021
@nCastle1 nCastle1 deleted the ncastle/search-2 branch December 1, 2021 00:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants