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

New sample: Define clustering feature reduction #1316

Merged
merged 16 commits into from
Nov 13, 2023
Merged

New sample: Define clustering feature reduction #1316

merged 16 commits into from
Nov 13, 2023

Conversation

duffh
Copy link
Collaborator

@duffh duffh commented Nov 3, 2023 •
edited by williambohrmann3
Loading

Description

Implemented new sample Custom feature clustering.

Type of change

  • New sample implementation

Platforms tested on

  • WPF .NET 8
  • WPF Framework
  • WinUI
  • MAUI WinUI
  • MAUI Android
  • MAUI iOS
  • MAUI MacCatalyst

Checklist

  • Self-review of changes
  • All changes work as expected on all affected platforms
  • There are no warnings related to changes
  • Code is commented and follows .NET conventions and standards
  • Codemaid and XAML styler extensions have been run on every changed file
  • No unrelated changes have been made to any other code or project files
  • Screenshots are correct size and display in description tab (800 x 600 on Windows, MAUI mobile platforms use the MAUI Windows screenshot)

williambohrmann3
williambohrmann3 reviewed Nov 3, 2023
View reviewed changes
Copy link
Collaborator

@williambohrmann3 williambohrmann3 left a comment

Choose a reason for hiding this comment

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

Nice, this is a great addition to our samples offering. Left a few minor nitpicks.

src/MAUI/Maui.Samples/Samples/Layers/CustomFeatureClustering/CustomFeatureClustering.xaml.cs Outdated Show resolved Hide resolved
src/MAUI/Maui.Samples/Samples/Layers/CustomFeatureClustering/CustomFeatureClustering.xaml.cs Outdated Show resolved Hide resolved
src/MAUI/Maui.Samples/Samples/Layers/CustomFeatureClustering/readme.md Outdated
@@ -0,0 +1,42 @@
# Custom feature clustering

Add client side custom feature reduction to a web map that does not have an existing defined feature reduction.
Copy link
Collaborator

Choose a reason for hiding this comment

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

A little bit confused by this wording, what is meant by "existing defined"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It was tricky to word initially, I'm essentially trying to say that this sample demonstrates how to add custom feature reduction in the application when it is not already defined on a web map.

So in this case "an existing defined feature reduction" is a feature reduction which exists and has been defined with some parameters.

Copy link
Collaborator

Choose a reason for hiding this comment

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

understood, thanks for clarifying

src/MAUI/Maui.Samples/Samples/Layers/CustomFeatureClustering/readme.md Outdated Show resolved Hide resolved
src/Samples.Shared/Resources/FeaturedSamples.xml Outdated Show resolved Hide resolved
src/MAUI/Maui.Samples/Samples/Layers/CustomFeatureClustering/CustomFeatureClustering.xaml Outdated
xmlns:esriUI="clr-namespace:Esri.ArcGISRuntime.Maui;assembly=Esri.ArcGISRuntime.Maui">
<Grid ColumnDefinitions="*, Auto" RowDefinitions="Auto, *">
<esriUI:MapView x:Name="MyMapView" Style="{DynamicResource EsriSampleGeoView}" />
<Border Style="{DynamicResource EsriSampleControlPanel}">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Control panel is rendering 90% off screen for me on iPhone XS.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll run this on iOS shortly to see if I can find a solution.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I should've added - this is an issue when rotating device orientation to landscape.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've tested it. As it works and displays ok in portrait I think it's ok for now as I don't think the fix is a small piece of effort.

williambohrmann3
williambohrmann3 approved these changes Nov 6, 2023
View reviewed changes
Copy link
Collaborator

@williambohrmann3 williambohrmann3 left a comment

Choose a reason for hiding this comment

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

thanks for addressing my nitpicks, LGTM.

src/MAUI/Maui.Samples/Samples/Layers/CustomFeatureClustering/CustomFeatureClustering.xaml Outdated
xmlns:esriUI="clr-namespace:Esri.ArcGISRuntime.Maui;assembly=Esri.ArcGISRuntime.Maui">
<Grid ColumnDefinitions="*, Auto" RowDefinitions="Auto, *">
<esriUI:MapView x:Name="MyMapView" Style="{DynamicResource EsriSampleGeoView}" />
<Border Style="{DynamicResource EsriSampleControlPanel}">
Copy link
Collaborator

Choose a reason for hiding this comment

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

I should've added - this is an issue when rotating device orientation to landscape.

src/MAUI/Maui.Samples/Samples/Layers/CustomFeatureClustering/readme.md Outdated
@@ -0,0 +1,42 @@
# Custom feature clustering

Add client side custom feature reduction to a web map that does not have an existing defined feature reduction.
Copy link
Collaborator

Choose a reason for hiding this comment

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

understood, thanks for clarifying

pMaske
pMaske reviewed Nov 6, 2023
View reviewed changes
Copy link
Member

@pMaske pMaske left a comment

Choose a reason for hiding this comment

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

Added few suggestions @duffh.
I also have feedback on sample name. I was wondering if we can name sample as DefineClusteringFeatureReduction instead of using word Custom . Happy to brainstorm for a better name, but word custom sounds like users can use something that's not esri defined.

src/MAUI/Maui.Samples/Samples/Layers/CustomFeatureClustering/CustomFeatureClustering.xaml.cs Outdated Show resolved Hide resolved
src/MAUI/Maui.Samples/Samples/Layers/CustomFeatureClustering/readme.md Outdated Show resolved Hide resolved
src/MAUI/Maui.Samples/Samples/Layers/CustomFeatureClustering/readme.md Outdated Show resolved Hide resolved
src/MAUI/Maui.Samples/Samples/Layers/CustomFeatureClustering/readme.md Outdated Show resolved Hide resolved
src/MAUI/Maui.Samples/Samples/Layers/CustomFeatureClustering/readme.md Outdated Show resolved Hide resolved
src/MAUI/readme.md Outdated Show resolved Hide resolved
src/WinUI/readme.md Outdated Show resolved Hide resolved
src/WinUI/ArcGIS.WinUI.Viewer/Samples/Layers/CustomFeatureClustering/readme.metadata.json Outdated Show resolved Hide resolved
src/MAUI/Maui.Samples/Samples/Layers/CustomFeatureClustering/readme.metadata.json Outdated Show resolved Hide resolved
@duffh duffh requested a review from pMaske November 7, 2023 13:55
@duffh duffh changed the title New sample: Custom feature clustering New sample: Define clustering feature reduction Nov 7, 2023
williambohrmann3
williambohrmann3 reviewed Nov 8, 2023
View reviewed changes
...amples/Samples/Layers/DefineClusteringFeatureReduction/DefineClusteringFeatureReduction.xaml Outdated
xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
xmlns:esriTK="clr-namespace:Esri.ArcGISRuntime.Toolkit.Maui;assembly=Esri.ArcGISRuntime.Toolkit.Maui"
xmlns:esriUI="clr-namespace:Esri.ArcGISRuntime.Maui;assembly=Esri.ArcGISRuntime.Maui">
<Grid ColumnDefinitions="*, Auto" RowDefinitions="Auto, *">
Copy link
Collaborator

@williambohrmann3 williambohrmann3 Nov 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Can you use the DynamicResource called EsriSampleContainer for this Grid? That will fix the landscape orientation issue once #1317 is completed.

pMaske
pMaske approved these changes Nov 8, 2023
View reviewed changes
Copy link
Member

@pMaske pMaske left a comment

Choose a reason for hiding this comment

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

One minor update in sample description, otherwise looks good

src/MAUI/Maui.Samples/Samples/Layers/DefineClusteringFeatureReduction/readme.md Outdated Show resolved Hide resolved
src/WPF/WPF.Viewer/Samples/Layers/DefineClusteringFeatureReduction/readme.md Outdated Show resolved Hide resolved
src/WinUI/ArcGIS.WinUI.Viewer/Samples/Layers/DefineClusteringFeatureReduction/readme.md Outdated Show resolved Hide resolved
@duffh duffh merged commit 223e4f9 into v.next Nov 13, 2023
2 checks passed
@duffh duffh deleted the hduff/custom-feature-clustering branch November 13, 2023 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@williambohrmann3 williambohrmann3 williambohrmann3 approved these changes

@pMaske pMaske pMaske approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@duffh @pMaske @williambohrmann3